Skip to content

Commit b491554

Browse files
committed
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. - ProxyDisclosureScanRuleUnitTest > Added a test to assert the new behavior. Signed-off-by: kingthorin <[email protected]>
1 parent d34392d commit b491554

File tree

4 files changed

+49
-1
lines changed

4 files changed

+49
-1
lines changed

addOns/ascanrulesBeta/CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
66
## Unreleased
77
### Changed
88
- Log exception details in Out of Band XSS scan rule.
9+
- 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).
910

1011
## [55] - 2024-09-02
1112
### Changed

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,12 @@ public void scan() {
331331
String proxyServer = PROXY_REQUEST_HEADERS.get(proxyHeaderPattern);
332332
Matcher proxyHeaderMatcher =
333333
proxyHeaderPattern.matcher(traceResponseBody);
334-
if (proxyHeaderMatcher.find()) {
334+
Matcher originalBodyMatcher =
335+
proxyHeaderPattern.matcher(
336+
getBaseMsg().getResponseBody().toString());
337+
// Ensure the original message didn't already have x-forward type
338+
// content
339+
if (!originalBodyMatcher.find() && proxyHeaderMatcher.find()) {
335340
String proxyHeaderName = proxyHeaderMatcher.group(1);
336341
proxyActuallyFound = true;
337342
LOGGER.debug(

addOns/ascanrulesBeta/src/main/javahelp/org/zaproxy/zap/extension/ascanrulesBeta/resources/help/contents/ascanbeta.html

+2
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,8 @@ <H2 id="id-40025">Proxy Disclosure</H2>
136136
<li>The presence or absence of any proxy-based components that might cause attacks against the application to be detected, prevented, or mitigated.</li>
137137
</ul>
138138
<p>
139+
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).
140+
<p>
139141
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>
140142
<br>
141143
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

+40
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,24 @@
1919
*/
2020
package org.zaproxy.zap.extension.ascanrulesBeta;
2121

22+
import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse;
2223
import static org.hamcrest.MatcherAssert.assertThat;
2324
import static org.hamcrest.Matchers.equalTo;
25+
import static org.hamcrest.Matchers.hasSize;
2426
import static org.hamcrest.Matchers.is;
2527

28+
import fi.iki.elonen.NanoHTTPD;
29+
import fi.iki.elonen.NanoHTTPD.IHTTPSession;
30+
import fi.iki.elonen.NanoHTTPD.Response;
2631
import java.util.Map;
32+
import org.apache.commons.httpclient.URIException;
2733
import org.junit.jupiter.api.Test;
34+
import org.junit.jupiter.params.ParameterizedTest;
35+
import org.junit.jupiter.params.provider.ValueSource;
36+
import org.parosproxy.paros.network.HttpMalformedHeaderException;
37+
import org.parosproxy.paros.network.HttpMessage;
2838
import org.zaproxy.addon.commonlib.CommonAlertTag;
39+
import org.zaproxy.zap.testutils.NanoServerHandler;
2940

3041
class ProxyDisclosureScanRuleUnitTest extends ActiveScannerTest<ProxyDisclosureScanRule> {
3142

@@ -57,4 +68,33 @@ void shouldReturnExpectedMappings() {
5768
tags.get(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getTag()),
5869
is(equalTo(CommonAlertTag.OWASP_2017_A06_SEC_MISCONFIG.getValue())));
5970
}
71+
72+
@ParameterizedTest
73+
@ValueSource(
74+
strings = {
75+
"X-Forwarded-For: 76.69.54.171", "X-Forwarded-For: 127.0.0.1",
76+
"X-Forwarded-Host: api.test.glaypen.garnercorp.com", "X-Forwarded-Port: 443",
77+
"X-Forwarded-Proto: https", "X-Forwarded-Scheme: https"
78+
})
79+
void shouldNotAlertIfOriginalHasXForwardContent(String header)
80+
throws HttpMalformedHeaderException, URIException {
81+
// Given
82+
String test = "/";
83+
nano.addHandler(
84+
new NanoServerHandler(test) {
85+
86+
@Override
87+
protected Response serve(IHTTPSession session) {
88+
String content = "<html>" + header + "</html>";
89+
return newFixedLengthResponse(
90+
Response.Status.OK, NanoHTTPD.MIME_HTML, content);
91+
}
92+
});
93+
HttpMessage msg = getHttpMessage(test);
94+
rule.init(msg, parent);
95+
// When
96+
rule.scan();
97+
// Then
98+
assertThat(alertsRaised, hasSize(equalTo(0))); // No messages sent
99+
}
60100
}

0 commit comments

Comments
 (0)