zap-extensions
zap-extensions copied to clipboard
ascanrules: status code heuristic for sqli to reduce false positives
Overview
Fixes sql injection false positives https://github.com/zaproxy/zaproxy/issues/8652 and https://github.com/zaproxy/zaproxy/issues/8653. The short summary is that the current response comparison logic just checks if the response bodies are the same or different. It's unaware of the response status codes. This change includes the status code as a heuristic when comparing responses for expression based and boolean based tests. The have to be the same status code for a sql injection alert to be raised.
Note that this potentially adds false negatives. It's possible that the different status code is a result of the confirm sql payload being evaluated. However, I've looked at over 50 different websites with alerts from this case and none of them were a real vulnerability. I think it's pretty unusual that the first sql payload is evaluated fine and the second confirm payload results in a totally different status code. Not impossible, but I think it has a very high noise to real finding ratio.
Some alternatives:
- set a lower alert confidence if there was a status code mismatch
- handle different error codes differently e.g. a 429 is really unlikely to be a sql injection but a 5xx is a little more suspicious
This change is built on top of [TODO]. Once that one is done I'll rebase, squash, and sign-off the resulting commit.
Related Issues
Fix zaproxy/zaproxy#8652 Fix zaproxy/zaproxy#8653
Checklist
- [ ] Update help
- [x] Update changelog
- [x] Run
./gradlew spotlessApplyfor code formatting - [x] Write tests
- [x] Check code coverage
- [ ] Sign-off commits
- [ ] Squash commits
- [x] Use a descriptive title
For more details, please refer to the developer rules and guidelines.
Checkmarx One – Scan Summary & Details – 8b977604-a502-4693-aea9-062020488795