From e0c29af11aba29e3fc052b329d3527d9a242663b Mon Sep 17 00:00:00 2001
From: kingthorin <kingthorin@users.noreply.github.com>
Date: Mon, 9 Sep 2024 07:21:37 -0400
Subject: [PATCH] ascanrulesBeta: Address possible FP in proxy detection rule

- CHANGELOG > Added change note.
- ascanbeta.html > added note about the new condition.
- ProxyDisclosureScanRule > Added condition to skip messages if they
have an "x-forward" type header to start with.
- ProxyDisclosureScanRuleUnitTest > Added a test to assert the new
behavior.

Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
---
 addOns/ascanrulesBeta/CHANGELOG.md             |  1 +
 .../ProxyDisclosureScanRule.java               |  6 ++++++
 .../resources/help/contents/ascanbeta.html     |  2 ++
 .../ProxyDisclosureScanRuleUnitTest.java       | 18 ++++++++++++++++++
 4 files changed, 27 insertions(+)

diff --git a/addOns/ascanrulesBeta/CHANGELOG.md b/addOns/ascanrulesBeta/CHANGELOG.md
index 33d62066a47..6473b5e57a9 100644
--- a/addOns/ascanrulesBeta/CHANGELOG.md
+++ b/addOns/ascanrulesBeta/CHANGELOG.md
@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 ## Unreleased
 ### Changed
 - Log exception details in Out of Band XSS scan rule.
+- The Proxy Disclosure scan rule will no longer process messages that have an X-Forward type header to start with, in order to reduce possible false positives (Issue 8556).
 
 ## [55] - 2024-09-02
 ### Changed
diff --git a/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRule.java b/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRule.java
index 22806d92d88..0109353d0e6 100644
--- a/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRule.java
+++ b/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRule.java
@@ -32,6 +32,7 @@
 import java.util.regex.Pattern;
 import org.apache.commons.httpclient.URI;
 import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.parosproxy.paros.Constant;
@@ -193,6 +194,11 @@ public void init() {
     @Override
     public void scan() {
         try {
+            if (StringUtils.containsIgnoreCase(
+                    getBaseMsg().getRequestHeader().getHeadersAsString(), "x-forward")) {
+                // If it has an x-forward type header to start with just skip it
+                return;
+            }
             // where's what we're going to do (roughly):
             // 1: If TRACE is enabled on the origin web server, we're going to use it, and the
             // "Max-Forwards" header to verify
diff --git a/addOns/ascanrulesBeta/src/main/javahelp/org/zaproxy/zap/extension/ascanrulesBeta/resources/help/contents/ascanbeta.html b/addOns/ascanrulesBeta/src/main/javahelp/org/zaproxy/zap/extension/ascanrulesBeta/resources/help/contents/ascanbeta.html
index 47d65b7a861..32fc0aa1cb7 100644
--- a/addOns/ascanrulesBeta/src/main/javahelp/org/zaproxy/zap/extension/ascanrulesBeta/resources/help/contents/ascanbeta.html
+++ b/addOns/ascanrulesBeta/src/main/javahelp/org/zaproxy/zap/extension/ascanrulesBeta/resources/help/contents/ascanbeta.html
@@ -136,6 +136,8 @@ <H2 id="id-40025">Proxy Disclosure</H2>
 <li>The presence or absence of any proxy-based components that might cause attacks against the application to be detected, prevented, or mitigated.</li>
 </ul>
 <p>
+Note: The rule will skip HTTP messages that have an "X-Foward" type header to start with (in order to reduce possible false positives).
+<p>
 Latest code: <a href="https://github.com/zaproxy/zap-extensions/blob/main/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRule.java">ProxyDisclosureScanRule.java</a>
 <br>
 Alert ID: <a href="https://www.zaproxy.org/docs/alerts/40025/">40025</a>.
diff --git a/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRuleUnitTest.java b/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRuleUnitTest.java
index 752814b569c..0dc133d0ee3 100644
--- a/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRuleUnitTest.java
+++ b/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRuleUnitTest.java
@@ -21,10 +21,15 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.is;
 
 import java.util.Map;
+import org.apache.commons.httpclient.URI;
+import org.apache.commons.httpclient.URIException;
 import org.junit.jupiter.api.Test;
+import org.parosproxy.paros.network.HttpMalformedHeaderException;
+import org.parosproxy.paros.network.HttpMessage;
 import org.zaproxy.addon.commonlib.CommonAlertTag;
 
 class ProxyDisclosureScanRuleUnitTest extends ActiveScannerTest<ProxyDisclosureScanRule> {
@@ -57,4 +62,17 @@ void shouldReturnExpectedMappings() {
                 tags.get(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getTag()),
                 is(equalTo(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getValue())));
     }
+
+    @Test
+    void shouldNotProcessIfOriginalHasXForwardHeader()
+            throws HttpMalformedHeaderException, URIException {
+        // Given
+        HttpMessage msg = new HttpMessage(new URI("https://example.org", false));
+        msg.getRequestHeader().addHeader("X-Forwarded-For", "127.0.0.1");
+        rule.init(msg, parent);
+        // When
+        rule.scan();
+        // Then
+        assertThat(httpMessagesSent, hasSize(equalTo(0))); // No messages sent
+    }
 }