Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ascanrulesBeta: Address possible FP in proxy detection rule #5718

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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");
thc202 marked this conversation as resolved.
Show resolved Hide resolved
}

@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
}
}