ascanrules: Reduce SQL Injection boolean-based false positives#7268
Open
Karl-Seryani wants to merge 1 commit intozaproxy:mainfrom
Open
ascanrules: Reduce SQL Injection boolean-based false positives#7268Karl-Seryani wants to merge 1 commit intozaproxy:mainfrom
Karl-Seryani wants to merge 1 commit intozaproxy:mainfrom
Conversation
Member
|
Great job! No new security vulnerabilities introduced in this pull requestUse @Checkmarx to interact with Checkmarx PR Assistant. |
Add a control request to the boolean-based SQL injection detection to verify page stability before raising alerts. After AND FALSE or OR TRUE differs from baseline, the scan re-sends the original parameter value. If the control response also differs, the page is unstable and the alert is raised at CONFIDENCE_LOW instead of CONFIDENCE_MEDIUM. This reduces false positives from pages with dynamic content (CSRF tokens, timestamps, view counters) while still surfacing potential findings for manual verification. Fixes zaproxy/zaproxy#9289 Signed-off-by: Karl Seryani <karlseryani@gmail.com>
780f971 to
c4df0cb
Compare
Contributor
Author
|
Updated to extract the duplicated control check logic into a checkPageStability() helper method. The AND FALSE and OR TRUE paths now share the same method. The no-data path stays inline since it uses string comparison instead of compareResponses(). |
This file contains hidden or 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
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.

Summary
CONFIDENCE_LOWinstead ofCONFIDENCE_MEDIUMFixes zaproxy/zaproxy#9289
Context
The boolean-based checks currently do two requests: AND TRUE (should match original) and AND FALSE (should differ). This can false-positive when the page has dynamic content (CSRF tokens, timestamps, view counters) that changes between requests. The expression-based check already has proper confirmation, so this applies a similar pattern to the boolean-based paths.
Rather than suppressing alerts entirely when instability is detected, the fix degrades confidence to LOW so the finding is still surfaced for manual verification.
Test plan
shouldAlertWithLowConfidenceIfControlRequestShowsPageIsUnstable- AND FALSE path with unstable pageshouldAlertIfControlConfirmsStablePage- stable page still alerts at MEDIUMshouldNotAlertIfNumericParameterStripsNonNumericInput- regression test for #9289shouldAlertWithLowConfidenceIfOrTrueControlRequestShowsPageIsUnstable- OR TRUE path with unstable pageshouldAlertWithLowConfidenceIfNoDataControlRequestShowsPageIsUnstable- no-data path with unstable page./gradlew :addOns:ascanrules:checkpasses