-
Notifications
You must be signed in to change notification settings - Fork 0
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
SQL injection detection #12
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Change some function and type names - Prepare for detecting SQL injection
…tection. Flesh another function out slightly more.
…es, and for Knex).
…rue before, I just never noticed this earlier)
…obust (although there are still some more cases to handle). Replace `getLocal` with `isLocal` again.
- Expand the test cases - Now inferring when assignment is happening correctly (it was very half-assed before, and I had some differing logic on how to handle it - both incomplete ways! - in the global variable detection code and the SQL injection detection code) - General improvements: supporting all assignment operator types, global variable detection can now handle one of the weird variable swap cases, and the stack frames are now sets again, instead of maps.
…ic worked out, it's quite elegant IMO. - I now just need to clean up a bit of the code, support more callsites, disallow format strings, and test some other binary operators on strings too. - Also replace all throws with a `panic` function that I made (following DRY!).
…s global (for easy tweaking in the future). Remove a comment, and add a TODO.
… detect SQL injection in a more clear way. More type renaming.
… allocation is happening anymore)
…ecorator set error checker mapping to evaluate different determinism checkers)
… raw SQL clients.
…ditional installation instructions later.
kraftp
reviewed
Jul 24, 2024
kraftp
approved these changes
Jul 25, 2024
CaspianA1
added a commit
to dbos-inc/dbos-docs
that referenced
this pull request
Jul 25, 2024
Going to merge this once [this](dbos-inc/eslint-plugin#12) PR is merged --------- Signed-off-by: Caspian <[email protected]> Co-authored-by: Qian Li <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Mostly done, but not quite; I just made a PR to run e2e tests and get feedback on my progress so far.
Here's a summary of my changes:
z = [y, y = z][0];
, and modifications separated by a comment, likex = 23 + x, y.a = 24 + x;
). This covers all assignment expression types, I believe, except for spread assignments, which I will need to implement at some point.Here's how I determine probable SQL injections:
ctxt.client.raw(myQuery)
, where the ORM client isKnex
), figure out if the value is reducible to a string literal, or concatenations of string literals.`${foo}, ${bar}, ${baz}`
) are also supported, as long as the values used to build it (foo
,bar
, andbaz
here) are also reducible to string literals. This does not include tagged template expressions, since those involve a function call, and my variable definition analysis stays within the scope of a singleTransaction
function.Transaction
function's parameters for a raw SQL query is not allowed either, since you don't know who will call that transaction except in certain cases.Here's what's left:
Knex
'sraw
function). Should be relatively easy.And things to think about after that: