Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Knex QueryBuilder toString is unreliable for producing cache keys for PG. #86

Open
willrust opened this issue Dec 16, 2022 · 1 comment

Comments

@willrust
Copy link

I believe this is related to knex issue 3553, but wanted to surface it here because of a potential security issue that could result.

For PG, query.toString() will omit bigint values, yet the actual query can succeed.

Here's an example of the resulting string where a bigint id is used:
select "id", "username" from "users" where "id" = '' limit 1
Notice that id = ''. So in this case, the first user queried will be returned for subsequent queries as the cache key is always the same.

Also, the Knex docs suggest that toString() should be used for debugging, but we're using it for something quite important.

@cvburgess
Copy link
Owner

Great callout @willrust - when I created this utility for my team we used UUIDs for IDs across our app, never integers of any kind.

If you can suggest a better path forward I would gladly welcome a PR.

Personally, for security, I implement RLS so the only records selected belong to the user currently authenticated. I don't have a great idea for this particular scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants