3 ms·
One of the things I appreciate about pocketbase is GG's attention to detail with respect to security. Is the string interpolation straight into the sql from th
by jddj 2y ago
One of the things I appreciate about pocketbase is GG's attention to detail with respect to security.
Is the string interpolation straight into the sql from the query string in your getting started docs (https://trailbase.io/getting-started/first-ui-app https://trailbase.io/getting-started/first-ui-app) safe? It smells a bit.
From a quick look (on mobile) I can see that the function takes parameters, but you're not using that.
- jagged-chisel 2y agoThere’s this whole thread about string handling. Does the Rust API into SQLite not support prepared statements?
- ansible 2y agoYou can insert arguments into an SQL statement with the (?1) style syntax. See the example here: https://docs.rs/rusqlite/latest/rusqlite/ https://docs.rs/rusqlite/latest/rusqlite/
- nyclounge 2y agono, but the guy can also look at postgres.js that already implmented alot of the sql`SELECT * FROM ${table_name}` Where there vars are escaped. It ONLY works for postgres, but I'm sure you can make it work with SQLite as well.
- jagged-chisel 2y agoIt does: https://docs.rs/rusqlite/latest/rusqlite/struct.Statement.html https://docs.rs/rusqlite/latest/rusqlite/struct.Statement.ht... Mucking about with SQL strings, to me, is akin to writing your own crypto: Don't. Unless you must (because, say, it doesn't exist yet.) Doing this is asking for security problems later. Trust that the SQLite team (or any other SQL engine) have more experience and have provided a correct interface. And if that interface isn't correct, don't just run off an make your own- contribute a fix so we can keep all the lessons learned together.
- klaussilveira 2y agoInteresting to see all these Rust and TS server-side projects fumbling on the same mistakes early PHP 4 was crucified for.
- chrismorgan 2y agoparsePath(…).query is a URLSearchParams, so those parameters are just strings, and it’s an untagged template string, so it is indeed a trivial SQL injection vector. And the params argument was right there! I don’t understand quite how you make a mistake like that, but it’s extremely worrying. This is fundamental, basic stuff. The fun aspects of this are that: (a) I could actually imagine trailbase.js contents that would make it not SQL injection: you could have parsePath(…).query.get(…) return objects with a toString() that escaped SQL. This would raise even more questions, and I was sure it wouldn’t be the case, but it’s possible. (b) You could make it work safely, converting interpolation into parameters, by using a tagged template string. This could require only a tiny change: return await query( sql`SELECT Owner, Aroma, Flavor, Acidity, Sweetness FROM coffee ORDER BY vec_distance_L2( embedding, '[${+aroma}, ${+flavor}, ${+acid}, ${+sweet}]') LIMIT 100` ); (You could even make it query`…`, but I think query(sql`…`) is probably wiser. As for the plusses I put in there, that’s to convert from strings to numbers.) This is a concept that’s definitely been done seriously. The first search result I found: https://github.com/blakeembrey/sql-template-tag https://github.com/blakeembrey/sql-template-tag.
- uwemaurer 2y agoit looks like that the query function has a second parameter with a "params" array. probably something like this would work then too (didn't test): return await query( `SELECT Owner, Aroma, Flavor, Acidity, Sweetness FROM coffee ORDER BY vec_distance_L2( embedding, '[?,?,?,?]') LIMIT 100`, [+aroma, +flavor, +acid, +sweet], ); it is nice because the query string is constant then and a prepared query could be cached..
- chrismorgan 2y agoSo we’re clear, the tagged template string approach gives you a string that uses parameters properly (see the example in the README of that project that does it); it’s perfectly compatible with prepared-statement caching. And actually on second consideration, you probably can’t use binding parameters here: you’re trying to slot numbers in as JSON values inside a string! Maybe you’d have to write vec_distance_L2(embedding, ?), with parameter JSON.stringify([+aroma, +flavor, +acid, +sweet]).