From e90863ca2cfde143598d033e49b58fe64a985307 Mon Sep 17 00:00:00 2001
From: thc202 <thc202@gmail.com>
Date: Tue, 17 Sep 2024 09:25:15 +0100
Subject: [PATCH] ascanrules: address FPs in scan rule 20017

Do not scan binary responses and responses that already contain PHP
source.

Fix zaproxy/zaproxy#8638.

Signed-off-by: thc202 <thc202@gmail.com>
---
 addOns/ascanrules/CHANGELOG.md                |  3 ++
 ...urceCodeDisclosureCve20121823ScanRule.java | 14 +++++++-
 ...DisclosureCve20121823ScanRuleUnitTest.java | 32 +++++++++++++++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md
index f9f4b850efd..010df41eab1 100644
--- a/addOns/ascanrules/CHANGELOG.md
+++ b/addOns/ascanrules/CHANGELOG.md
@@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Maintenance changes.
 - The Spring Actuator Scan Rule now includes example alert functionality for documentation generation purposes (Issue 6119).
 
+### Fixed
+- Address false positives with Source Code Disclosure - CVE-2012-1823 scan rule, by not scanning binary responses and responses that already contain PHP source (Issue 8638).
+
 ## [67] - 2024-07-22
 
 ### Changed
diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRule.java
index 6ad9e1abe27..7825cd8a154 100644
--- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRule.java
+++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRule.java
@@ -121,13 +121,18 @@ public String getReference() {
     public void scan() {
         try {
 
-            if (!getBaseMsg().getResponseHeader().isText()) {
+            if (!getBaseMsg().getResponseHeader().isText()
+                    || ResourceIdentificationUtils.responseContainsControlChars(getBaseMsg())) {
                 return; // Ignore images, pdfs, etc.
             }
             if (getAlertThreshold() != AlertThreshold.LOW
                     && ResourceIdentificationUtils.isJavaScript(getBaseMsg())) {
                 return;
             }
+
+            if (isEvidenceInOriginalResponse()) {
+                return;
+            }
             // at Low or Medium strength, do not attack URLs which returned "Not Found"
             AttackStrength attackStrength = getAttackStrength();
             if ((attackStrength == AttackStrength.LOW || attackStrength == AttackStrength.MEDIUM)
@@ -181,6 +186,13 @@ public void scan() {
         }
     }
 
+    private boolean isEvidenceInOriginalResponse() {
+        var response = getBaseMsg().getResponseBody().toString();
+        String responseBodyDecoded = new Source(response).getRenderer().toString();
+        return PHP_PATTERN1.matcher(responseBodyDecoded).matches()
+                || PHP_PATTERN2.matcher(responseBodyDecoded).matches();
+    }
+
     private AlertBuilder buildAlert(String otherInfo) {
         return newAlert()
                 .setConfidence(Alert.CONFIDENCE_MEDIUM)
diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRuleUnitTest.java
index c55883f338b..7d26407e81b 100644
--- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRuleUnitTest.java
+++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRuleUnitTest.java
@@ -31,6 +31,8 @@
 import java.util.Map;
 import org.apache.commons.text.StringEscapeUtils;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import org.parosproxy.paros.core.scanner.Alert;
 import org.parosproxy.paros.core.scanner.Plugin.AlertThreshold;
 import org.parosproxy.paros.core.scanner.Plugin.AttackStrength;
@@ -253,6 +255,36 @@ protected Response serve(IHTTPSession session) {
         assertThat(alertsRaised, hasSize(0));
     }
 
+    @ParameterizedTest
+    @ValueSource(strings = {PHP_SOURCE_TAGS, PHP_SOURCE_ECHO_TAG})
+    void shouldNotScanIfPhpSourceWasAlreadyPresentInResponse(String source) throws Exception {
+        // Given
+        var response =
+                "<html><body>PHP Tutorial: <code>"
+                        + StringEscapeUtils.escapeHtml4(source)
+                        + "</code></body></html>";
+        HttpMessage message = getHttpMessage("GET", "/", response);
+        rule.init(message, parent);
+        // When
+        rule.scan();
+        // Then
+        assertThat(httpMessagesSent, hasSize(0));
+        assertThat(alertsRaised, hasSize(0));
+    }
+
+    @Test
+    void shouldNotScanBinaryResponse() throws Exception {
+        // Given
+        var response = "�PNG\n\n";
+        HttpMessage message = getHttpMessage("GET", "/", response);
+        rule.init(message, parent);
+        // When
+        rule.scan();
+        // Then
+        assertThat(httpMessagesSent, hasSize(0));
+        assertThat(alertsRaised, hasSize(0));
+    }
+
     @Test
     void shouldAlertIfPhpEchoTagsWereDisclosedInResponseBody() throws Exception {
         // Given