ascanrulesBeta: Skip error responses in SourceCodeDisclosureFileInclusion

[Arunodoy18]

Description

Fixes zaproxy/zaproxy#9184

The SourceCodeDisclosureFileInclusionScanRule (ID 43) generates false positives when attack responses return 4xx/5xx status codes. Error pages naturally differ from success pages, and diffing them against the baseline gives misleading match percentages.

Root cause

The rule sends attack payloads (e.g. path traversal attempts) and compares the response body against a random-parameter baseline using DiceMatcher. If the server returns a 4xx error page for the attack (different from the random baseline), the diff exceeds the threshold and the rule falsely flags it as source code disclosure — especially when the URL has no file extension (dataMatchesExtension returns true when fileExtension == null).

Reported scenario: A GraphQL endpoint returns 200 for valid queries but 4xx for invalid ones. The rule extracts graph-ql from the URL path, sends it as a parameter value, receives a 4xx error page that differs from the baseline, and raises a false positive.

Fix

Add isClientError()/isServerError() checks on each attack response, immediately after sendAndReceive, in both:

1. The source file inclusion loop (line ~311)
2. The WAR/EAR file inclusion loop (line ~399)

If the attack response is a 4xx or 5xx, the iteration is skipped (continue). This mirrors the existing check on the base message at the start of the scan() method.

Why this is safe

• Real source code disclosure via file inclusion returns 200 (the server successfully served the included file)
• 4xx/5xx means the server rejected the request — no file was included
• The rule already applies isClientError/isServerError to the base message
• All 28 existing tests pass, plus 1 new test

New test

shouldNotAlertWhenAttackResponseIsClientError — uses a ClientErrorFiHandler that returns 200 for original/random requests but 404 for attack payloads, reproducing the exact scenario from the issue.

Checklist

• Tests pass (28/28 + 1 new)
• Spotless formatting verified
• Minimal change (2 insertion points, same pattern)
• No detection regression

[psiinon]

Checkmarx One – Scan Summary & Details – 77ccc844-70c6-49bb-9503-c2b46c83959d

Great job! No new security vulnerabilities introduced in this pull request

---

Use @Checkmarx to interact with Checkmarx PR Assistant.
Examples:
@Checkmarx how are you able to help me?
@Checkmarx rescan this PR

[jsoref]

The changes here look good and seem likely to address my false positives. 👍

[jsoref]

Speaking personally (and not as a member/maintainer, as I'm not), I'd rather have a isClientOrServerError() and use it both here and below. -- It makes it easier to quickly determine that the same logic is applied in both places.

[Arunodoy18]

Thanks for the suggestion! I've extracted an isClientOrServerError() helper and updated all three call sites to use it. Force-pushed.