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 "x-forward" type content to start with. Removed misleading Attack
text from the Alert.
- ProxyDisclosureScanRuleUnitTest > Added a test to assert the new
behavior.

Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>

Author: kingthorin

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 alert on HTTP messages that have "X-Forward" type content to start with, in order to reduce possible false positives (Issue 8556). The misleading Attack string for the Alerts was also removed.
 
 ## [55] - 2024-09-02
 ### Changed

addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRule.java

@@ -331,7 +331,12 @@ public void scan() {
                             String proxyServer = PROXY_REQUEST_HEADERS.get(proxyHeaderPattern);
                             Matcher proxyHeaderMatcher =
                                     proxyHeaderPattern.matcher(traceResponseBody);
-                            if (proxyHeaderMatcher.find()) {
+                            Matcher originalBodyMatcher =
+                                    proxyHeaderPattern.matcher(
+                                            getBaseMsg().getResponseBody().toString());
+                            // Ensure the original message didn't already have x-forward type
+                            // content
+                            if (!originalBodyMatcher.find() && proxyHeaderMatcher.find()) {
                                 String proxyHeaderName = proxyHeaderMatcher.group(1);
                                 proxyActuallyFound = true;
                                 LOGGER.debug(
@@ -752,7 +757,6 @@ public void scan() {
                                 Constant.messages.getString(
                                         MESSAGE_PREFIX + "desc",
                                         step2numberOfNodes - 1 + silentProxySet.size()))
-                        .setAttack(getAttack())
                         .setOtherInfo(extraInfo)
                         .setMessage(getBaseMsg())
                         .raise();
@@ -773,10 +777,6 @@ private String getPath(URI uri) {
         return "/";
     }
 
-    private String getAttack() {
-        return Constant.messages.getString(MESSAGE_PREFIX + "attack");
-    }
-
     @Override
     public int getRisk() {
         return Alert.RISK_MEDIUM;

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 not alert on HTTP messages that have "X-Foward" type content 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>.

addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ProxyDisclosureScanRuleUnitTest.java

@@ -19,13 +19,24 @@
  */
 package org.zaproxy.zap.extension.ascanrulesBeta;
 
+import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse;
 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 fi.iki.elonen.NanoHTTPD;
+import fi.iki.elonen.NanoHTTPD.IHTTPSession;
+import fi.iki.elonen.NanoHTTPD.Response;
 import java.util.Map;
+import org.apache.commons.httpclient.URIException;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.parosproxy.paros.network.HttpMalformedHeaderException;
+import org.parosproxy.paros.network.HttpMessage;
 import org.zaproxy.addon.commonlib.CommonAlertTag;
+import org.zaproxy.zap.testutils.NanoServerHandler;
 
 class ProxyDisclosureScanRuleUnitTest extends ActiveScannerTest<ProxyDisclosureScanRule> {
 
@@ -57,4 +68,33 @@ void shouldReturnExpectedMappings() {
                 tags.get(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getTag()),
                 is(equalTo(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getValue())));
     }
+
+    @ParameterizedTest
+    @ValueSource(
+            strings = {
+                "X-Forwarded-For: 76.69.54.171", "X-Forwarded-For: 127.0.0.1",
+                "X-Forwarded-Host: api.test.glaypen.garnercorp.com", "X-Forwarded-Port: 443",
+                "X-Forwarded-Proto: https", "X-Forwarded-Scheme: https"
+            })
+    void shouldNotAlertIfOriginalHasXForwardContent(String header)
+            throws HttpMalformedHeaderException, URIException {
+        // Given
+        String test = "/";
+        nano.addHandler(
+                new NanoServerHandler(test) {
+
+                    @Override
+                    protected Response serve(IHTTPSession session) {
+                        String content = "<html>" + header + "</html>";
+                        return newFixedLengthResponse(
+                                Response.Status.OK, NanoHTTPD.MIME_HTML, content);
+                    }
+                });
+        HttpMessage msg = getHttpMessage(test);
+        rule.init(msg, parent);
+        // When
+        rule.scan();
+        // Then
+        assertThat(alertsRaised, hasSize(equalTo(0))); // No messages sent
+    }
 }