Skip to content

Reduce false positives across all 15 checks (v2.1.0) - #5

Merged
liam-portswigger merged 1 commit into
mainfrom
fp-reduction
Jun 15, 2026
Merged

Reduce false positives across all 15 checks (v2.1.0)#5
liam-portswigger merged 1 commit into
mainfrom
fp-reduction

Conversation

@liam-portswigger

Copy link
Copy Markdown
Owner

Why

A deliberate false-positive audit (all 15 checks, each claim adversarially verified) found a systemic pattern: checks firing on a single signal without confirming the premise. 29 confirmed FPs clustered into four root causes — fixed here.

Root causes addressed

1. No baseline comparison (active checks). New HttpUtils.baselineContains() — the original rr.response() is the baseline, so no extra request is needed. A marker only counts when present in the payload response AND absent from baseline.

  • SSRF: skip markers already in baseline; ignore 4xx / rejection responses (OAuth echoing a rejected redirect_uri ≠ SSRF).
  • Injection: SQL-error / command-output markers must be new vs baseline (kills /bin/bash-in-docs, syntax error-on-every-error-page). XSS now requires HTML Content-Type.
  • Auth bypass: success markers must be new vs the baseline login response and not co-occur with a failure marker; removed broad logged/authenticated.

2. Status-only success. New HttpUtils.looksRejected() — a 2xx with an error/"not found"/"forbidden" body isn't success.

  • BrokenObjectAuth: ID-manipulation + enumeration now run only on authenticated requests (was firing Critical on public /articles/{id} — the worst FP found).
  • FunctionLevelAuth: same rejection guard.
  • MethodFuzzing/TRACE: sends a marker header, requires it echoed back (XST needs reflection, not just a 200).

3. Keyword matches overclaiming (passive).

  • ExcessiveDataExposure: benign-field exceptions (content_hash, etag, password_expiry_days); large-array suppressed when paginated.
  • BusinessFlow: removed bare challenge marker (matched marketing copy).
  • InventoryManagement: debug-endpoint Certain → Tentative/Low, reworded.
  • UnsafeApiConsumption: webhook keywords narrowed (dropped /callback, /notify, /event).

4. Threat-model blind spots.

  • BrokenAuth: API-key-over-HTTP skips loopback hosts.
  • SecurityMisconfig (CORS reflected): High/Firm → Medium/Tentative — a passive check can't tell validated reflection from blind.

5. ParameterPollution: status-code change is the primary signal; body-length path now measures the endpoint's natural jitter and only fires when the pollution delta clearly exceeds it.

Diff

22 files, +417 / −79. Detection logic only; no architectural change. Build verified clean locally; CI runs on this PR.

Test plan

  • mvn clean package clean.
  • CI green.
  • Load burp-api-scanner-2.1.0.jar in Burp; re-run the ginandjuice / brokencrystals scans and confirm the previously-flagged FPs (API-over-HTTP redirect, public-endpoint BOLA, etc.) no longer fire while genuine findings still do.

Releases as v2.1.0 (behavioural change, not a patch).

A deliberate false-positive audit (15 checks, adversarially verified)
found a systemic pattern: checks firing on a single signal without
confirming the premise. Confirmed FPs clustered into four root causes,
all addressed here.

1. No baseline comparison (active checks)
   New HttpUtils.baselineContains(): the original rr.response() IS the
   baseline, so no extra request is needed. A marker only counts as
   evidence when it's present in the payload response AND absent from
   the baseline.
   - SsrfCheck: skip payloads whose marker is already in the baseline;
     ignore 4xx / rejection responses (OAuth echoing a rejected
     redirect_uri is not SSRF).
   - InjectionCheck: SQL-error and command-output markers must be NEW
     vs baseline (kills "/bin/bash in API docs", "syntax error on every
     error page"). XSS now requires an HTML Content-Type — a JSON API
     echoing the payload as a string value is not exploitable.
   - AuthBypassTester: success markers must be new vs the baseline login
     response and not co-occur with a failure marker. Removed broad
     "logged"/"authenticated" markers that matched "not authenticated".

2. Status-only success (no body inspection)
   New HttpUtils.looksRejected(): a 2xx carrying an error / "not found"
   / "forbidden" body is the server refusing access with a sloppy
   status code, not success.
   - BrokenObjectAuthCheck: ID-manipulation and enumeration now run
     ONLY on authenticated requests — firing Critical on public
     /articles/{id} endpoints was the worst FP in the audit. Plus
     looksRejected guard on the success check.
   - FunctionLevelAuthCheck: looksRejected guard on the privileged-
     access check.
   - MethodFuzzingCheck (TRACE): now sends a marker header and requires
     it to be echoed back — XST is only real if TRACE reflects the
     request. A canned 200 from a proxy no longer fires.

3. Keyword matches overclaiming (passive)
   - ExcessiveDataExposure: benign-field exception list (content_hash,
     etag, password_expiry_days, …); large-array finding suppressed
     when pagination headers/params are present.
   - BusinessFlow: removed the bare word "challenge" from anti-
     automation markers (matched marketing copy and wrongly suppressed
     real findings); kept specific CAPTCHA/Turnstile tokens.
   - InventoryManagement: debug/management-endpoint finding downgraded
     Certain -> Tentative (Low), reworded to note docs/health probes
     are often intentional.
   - UnsafeApiConsumption: webhook keyword set narrowed to
     webhook-specific paths (dropped /callback, /notify, /event).

4. Threat-model blind spots
   - BrokenAuth: API-key-over-HTTP skips loopback hosts (no
     intermediary on localhost) via HttpUtils.isLoopbackHost().
   - SecurityMisconfig (CORS reflected origin): downgraded High/Firm ->
     Medium/Tentative and reworded — a single passive observation can't
     distinguish validated allow-list reflection from blind reflection.

5. ParameterPollution: status-code change is now the primary (clean)
   signal; the body-length-delta path measures the endpoint's natural
   jitter (one unmodified re-request) and only fires when the pollution
   delta clearly exceeds it — removes timestamp/UUID and verbose-error
   FPs.

Docs: VALIDATION_GUIDE CERTAIN table corrected for baseline diffing and
the XSS/HTTP confidence changes; version bumped to 2.1.0 across pom,
banner, CI, and docs. Build verified clean.
@liam-portswigger
liam-portswigger merged commit 8639903 into main Jun 15, 2026
1 check passed
@liam-portswigger
liam-portswigger deleted the fp-reduction branch June 15, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant