ascanrulesBeta: Address possible FP in proxy detection rule

- CHANGELOG > Added change note.
- ProxyDisclosureScanRule > Added condition to skip messages if they
have evidence content to start with. Removed misleading Attack
text from the Alert.
- ProxyDisclosureScanRuleUnitTest > Added a test to assert the new
behavior.

Signed-off-by: kingthorin <[email protected]>

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 evidence 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 evidence 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/resources/org/zaproxy/zap/extension/ascanrulesBeta/resources/Messages.properties

@@ -125,7 +125,6 @@ ascanbeta.noanticsrftokens.name = Absence of Anti-CSRF Tokens
 ascanbeta.oobxss.name = Out of Band XSS
 ascanbeta.oobxss.skipped = no Active Scan OAST service is selected.
 
-ascanbeta.proxydisclosure.attack = TRACE, OPTIONS methods with 'Max-Forwards' header. TRACK method.
 ascanbeta.proxydisclosure.desc = {0} proxy server(s) were detected or fingerprinted. This information helps a potential attacker to determine\n- A list of targets for an attack against the application.\n - Potential vulnerabilities on the proxy servers that service the application.\n - The presence or absence of any proxy-based components that might cause attacks against the application to be detected, prevented, or mitigated.
 ascanbeta.proxydisclosure.extrainfo.proxyserver = - {0}
 ascanbeta.proxydisclosure.extrainfo.proxyserver.header = Using the TRACE, OPTIONS, and TRACK methods, the following proxy servers have been identified between ZAP and the application/web server:

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,34 @@ 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: 127.0.0.1",
+                "X-Forwarded-Port: 443",
+                "X-Forwarded-Proto: https",
+                "Via: 1.1 vegur"
+            })
+    void shouldNotAlertIfOriginalHasEvidence(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
+    }
 }