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.
- 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 <kingthorin@users.noreply.github.com>
  • Loading branch information
kingthorin committed Sep 18, 2024
1 parent fed220d commit e4cf659
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 7 deletions.
1 change: 1 addition & 0 deletions addOns/ascanrulesBeta/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed
- Log exception details in Out of Band XSS scan rule.
- Maintenance changes.
- 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -752,7 +757,6 @@ public void scan() {
Constant.messages.getString(
MESSAGE_PREFIX + "desc",
step2numberOfNodes - 1 + silentProxySet.size()))
.setAttack(getAttack())
.setOtherInfo(extraInfo)
.setMessage(getBaseMsg())
.raise();
Expand All @@ -773,10 +777,6 @@ private static String getPath(URI uri) {
return "/";
}

private String getAttack() {
return Constant.messages.getString(MESSAGE_PREFIX + "attack");
}

@Override
public int getRisk() {
return Alert.RISK_MEDIUM;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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> {

Expand Down Expand Up @@ -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
}
}

0 comments on commit e4cf659

Please sign in to comment.