-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore(detectors): Prevent Query Injection detector from running on SQL queries #96690
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #96690 +/- ##
===========================================
+ Coverage 66.12% 80.14% +14.01%
===========================================
Files 8465 8483 +18
Lines 372806 379892 +7086
Branches 24205 24205
===========================================
+ Hits 246521 304452 +57931
+ Misses 125912 75067 -50845
Partials 373 373 |
|
||
sql_keywords = ("SELECT", "UPDATE", "INSERT") | ||
if any(description.upper().startswith(keyword) for keyword in sql_keywords): | ||
return False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: SQL Exclusion Flaws: DELETE Missing, Whitespace Ignored
The sql_keywords
tuple, intended to exclude common SQL queries from query injection detection, is missing "DELETE", a fundamental DML operation. This omission means "DELETE" queries are not excluded, leading to inconsistent behavior. Additionally, the keyword check does not account for leading whitespace in the span description, allowing queries like " SELECT * FROM users" to bypass the exclusion filter and still be processed by the detector, defeating its purpose of preventing false positives for common SQL operations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
worth adding a test?
fixes an edge case where some oddly formed SQL requests were getting picked up by the query injection detector (not the sql injection one)