Skip to content

Commit

Permalink
ascanrulesBeta: Address possible FP in proxy detection rule
Browse files Browse the repository at this point in the history
- 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>
  • Loading branch information
kingthorin committed Sep 9, 2024
1 parent d34392d commit e0c29af
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 0 deletions.
1 change: 1 addition & 0 deletions addOns/ascanrulesBeta/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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
}
}

0 comments on commit e0c29af

Please sign in to comment.