ascanrules: reduce false positives in boolean-based SQLi check - #7588
ascanrules: reduce false positives in boolean-based SQLi check#7588Char0n1507 wants to merge 3 commits into
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
…#9289) - Raise OR-TRUE response size threshold from 1.2x to 1.4x - Add minimum absolute size difference guard (500 bytes) - Prevents false positives on dynamic pages with variable content such as ads, timestamps, and session tokens
0b79e50 to
6219569
Compare
|
I have read the CLA Document and I hereby sign the CLA |
|
Fixed Issues (1)
Great job! The following issues were fixed in this Pull Request
Communicate with Checkmarx by submitting a PR comment with @Checkmarx followed by one of the supported commands. Learn about the supported commands here. |
|
Thanks for the review! Happy to make any adjustments needed. |
|
Why 500? That just makes things be big to start with. 500 is 40% of 1,250 so that assumes the response is over a meg to start with. I don't think this is a good mechanic. |
Replace the hardcoded 500-byte absolute difference guard with a relative check (20% of original response length), so the guard scales with response size rather than assuming responses are over 1MB. Add unit tests covering: - OR-TRUE ratio below 1.4x threshold (no alert) - OR-TRUE ratio just under boundary (no alert) - OR-TRUE ratio and diff both exceed thresholds (alert)
|
Good point on the 500 bytes I reconsidered and you're right, it's not a sound mechanic. I also looked at replacing it with a relative guard (20% of normalLen), but realized that's mathematically redundant: if ratio >= 1.4x then absoluteDiff >= 40% of normalLen already, so the second condition can never actually block anything the ratio check doesn't already catch. So I've simplified to just the ratio threshold alone (1.4x). That's the correct and sufficient fix. I've also added three unit tests to SqlInjectionScanRuleUnitTest.java:
|
Drop the redundant absolute byte guard — since ratio >= 1.4x already implies diff >= 40% of normalLen, the second condition added no filtering value. The ratio threshold alone (raised from 1.2x to 1.4x) is the correct and sufficient fix for #9289.
thc202
changed the title
3 participants
Summary
Reduces false positives in the boolean-based (OR TRUE) SQL injection check in
testBooleanBasedNoDataSqlInjection.Problem
Fixes zaproxy/zaproxy#9289
The check fires when the OR TRUE response body is >20% larger than the original.
Dynamic pages (ads, timestamps, session tokens, random content) naturally vary
by this amount, producing false positives on non-vulnerable endpoints.
Fix
Testing
20% size difference) is removed from the filenormalLen,orTrueLen,absoluteDiff) are presentifblock is completely unchanged