parsePath(…).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.
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:
(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.