Skip to content

Commit

Permalink
pscanrules: Big Redirect rule - Also alert on multiple HREFs in body
Browse files Browse the repository at this point in the history 
- BigRedirectScanRule > Add further functionality to check if the
response contains multiple HREFs, example alerts, and Alert Refs.
- BigRedirectScanRuleUnitTest > Add tests for new behavior, example
alerts, and alert refs.
- CHANGELOG > Add change notes.
- Messages.properties > Add supporting resources.
- pscanrules.html > Update help entry.

Signed-off-by: kingthorin <kingthorin@users.noreply.github.com>
  • Loading branch information
@kingthorin
kingthorin committed Dec 10, 2023
1 parent 58fc06e commit 1375f98
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 31 deletions.
4 changes: 3 additions & 1 deletion addOns/pscanrules/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ All notable changes to this add-on will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).

## Unreleased

### Changed
- The Big Redirect scan rule will now also alert on responses that have multiple HREFs (idea from xnl-h4ck3r).
- It also now includes example alert and alert reference functionality for documentation generation purposes (Issues 6119, 7100, and 8189).

## [53] - 2023-11-30
### Changed
Expand Down
79 changes: 59 additions & 20 deletions ...s/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/BigRedirectsScanRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@
*/
package org.zaproxy.zap.extension.pscanrules;

import java.util.List;
import java.util.Map;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import net.htmlparser.jericho.Source;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.parosproxy.paros.Constant;
import org.parosproxy.paros.core.scanner.Alert;
import org.parosproxy.paros.network.HttpHeader;
import org.parosproxy.paros.network.HttpMessage;
import org.parosproxy.paros.network.HttpResponseHeader;
import org.parosproxy.paros.network.HttpStatusCode;
import org.zaproxy.addon.commonlib.CommonAlertTag;
import org.zaproxy.zap.extension.pscan.PluginPassiveScanner;
Expand All @@ -53,8 +56,7 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
if (HttpStatusCode.isRedirection(msg.getResponseHeader().getStatusCode())
&& msg.getResponseHeader().getStatusCode() != 304) { // This response is a redirect
int responseLocationHeaderURILength = 0;
String locationHeaderValue =
msg.getResponseHeader().getHeader(HttpResponseHeader.LOCATION);
String locationHeaderValue = msg.getResponseHeader().getHeader(HttpHeader.LOCATION);
if (locationHeaderValue != null) {
responseLocationHeaderURILength = locationHeaderValue.length();
} else { // No location header found
Expand All @@ -70,22 +72,22 @@ public void scanHttpResponseReceive(HttpMessage msg, int id, Source source) {
// Check if response is bigger than predicted
if (responseBodyLength > predictedResponseSize) {
// Response is larger than predicted so raise an alert
newAlert()
.setRisk(Alert.RISK_LOW)
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setDescription(getDescription())
.setOtherInfo(
createBigAlert(
Constant.messages.getString(
MESSAGE_PREFIX + "extrainfo",
responseLocationHeaderURILength,
locationHeaderValue,
predictedResponseSize,
responseBodyLength))
.setSolution(getSolution())
.setReference(getReference())
.setCweId(201)
.setWascId(13)
.raise();
} else {
Pattern pattern = Pattern.compile("href", Pattern.CASE_INSENSITIVE);
Matcher matcher = pattern.matcher(msg.getResponseBody().toString());
long hrefCount = matcher.results().count();
if (hrefCount > 1) {
// Response more than one HREF so raise an alert
createMultiAlert(String.valueOf(hrefCount)).raise();
}
}
}
}
Expand All @@ -106,6 +108,46 @@ private int getPredictedResponseSize(int redirectURILength) {
return predictedResponseSize;
}

private AlertBuilder createBaseAlert() {
return newAlert()
.setRisk(Alert.RISK_LOW)
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setCweId(201)
.setWascId(13);
}

private AlertBuilder createBigAlert(String otherinfo) {
return createBaseAlert()
.setDescription(Constant.messages.getString(MESSAGE_PREFIX + "desc"))
.setOtherInfo(otherinfo)
.setSolution(getSolution())
.setAlertRef(String.valueOf(PLUGIN_ID) + "-1");
}

private AlertBuilder createMultiAlert(String otherinfo) {
return createBaseAlert()
.setName(Constant.messages.getString(MESSAGE_PREFIX + "multi.name"))
.setDescription(Constant.messages.getString(MESSAGE_PREFIX + "multi.desc"))
.setOtherInfo(
Constant.messages.getString(MESSAGE_PREFIX + "multi.extrainfo", otherinfo))
.setSolution(getSolution())
.setAlertRef(String.valueOf(PLUGIN_ID) + "-2");
}

@Override
public List<Alert> getExampleAlerts() {
return List.of(
createBigAlert(
Constant.messages.getString(
MESSAGE_PREFIX + "extrainfo",
18,
"http://example.com",
318,
319))
.build(),
createMultiAlert("3").build());
}

@Override
public int getPluginId() {
return PLUGIN_ID;
Expand All @@ -116,20 +158,17 @@ public String getName() {
return Constant.messages.getString(MESSAGE_PREFIX + "name");
}

private String getDescription() {
return Constant.messages.getString(MESSAGE_PREFIX + "desc");
}

private String getSolution() {
return Constant.messages.getString(MESSAGE_PREFIX + "soln");
}

private String getReference() {
return Constant.messages.getString(MESSAGE_PREFIX + "refs");
}

@Override
public Map<String, String> getAlertTags() {
return ALERT_TAGS;
}

public String getHelpLink() {
return "https://www.zaproxy.org/docs/desktop/addons/passive-scan-rules/#id-"
+ getPluginId();
}
}
4 changes: 3 additions & 1 deletion ...ain/javahelp/org/zaproxy/zap/extension/pscanrules/resources/help/contents/pscanrules.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,13 @@ <H2>Application Errors</H2>
<p>
Latest code: <a href="https://github.com/zaproxy/zap-extensions/blob/main/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/ApplicationErrorScanRule.java">ApplicationErrorScanRule.java</a>

<H2>Big Redirect Detected (Potential Sensitive Information Leak)</H2>
<H2 id="id-10044">Big Redirect Detected (Potential Sensitive Information Leak)</H2>
This check predicts the size of various redirect type responses and generates an alert if the response is greater than the predicted size.
A large redirect response may indicate that although a redirect has taken place the page actually contained content (which may reveal sensitive information, PII, etc.).
It will also raise an alert if the body of the redirect response contains multiple HREFs.
<p>
Latest code: <a href="https://github.com/zaproxy/zap-extensions/blob/main/addOns/pscanrules/src/main/java/org/zaproxy/zap/extension/pscanrules/BigRedirectsScanRule.java">BigRedirectsScanRule.java</a>
Alert ID: <a href="https://www.zaproxy.org/docs/alerts/10044/">10044</a>.

<H2>Cache Control</H2>
Checks "Cache-Control" response headers against general industry best practice settings for protection of sensitive content.<br>
Expand Down
4 changes: 3 additions & 1 deletion ...les/src/main/resources/org/zaproxy/zap/extension/pscanrules/resources/Messages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ pscanrules.authenticationcredentialscaptured.soln = Use HTTPS, and use a secure
pscanrules.bigredirects.desc = The server has responded with a redirect that seems to provide a large response. This may indicate that although the server sent a redirect it also responded with body content (which may include sensitive details, PII, etc.).
pscanrules.bigredirects.extrainfo = Location header URI length: {0} [{1}].\nPredicted response size: {2}.\nResponse Body Length: {3}.
pscanrules.bigredirects.multi.desc = The server has responded with a redirect that seems to provide multiple links. This may indicate that although the server sent a redirect it also responded with body content (which may include sensitive details, PII, etc.).
pscanrules.bigredirects.multi.extrainfo = The response contained {0} occurrences of "HREF".
pscanrules.bigredirects.multi.name = Strange Redirect Detected (Potential Sensitive Information Leak)
pscanrules.bigredirects.name = Big Redirect Detected (Potential Sensitive Information Leak)
pscanrules.bigredirects.refs =
pscanrules.bigredirects.soln = Ensure that no sensitive information is leaked via redirect responses. Redirect responses should have almost no content.
pscanrules.cachecontrol.desc = The cache-control header has not been set properly or is missing, allowing the browser and proxies to cache content. For static assets like css, js, or image files this might be intended, however, the resources should be reviewed to ensure that no sensitive content will be cached.
Expand Down
63 changes: 55 additions & 8 deletions ...ules/src/test/java/org/zaproxy/zap/extension/pscanrules/BigRedirectsScanRuleUnitTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static org.hamcrest.Matchers.is;

import java.io.IOException;
import java.util.List;
import java.util.Map;
import org.apache.commons.httpclient.URI;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -82,7 +83,28 @@ void givenRedirectHeadersWithLargeBodyThenAlertRaised() {

// Then
assertThat(alertsRaised.size(), is(1));
assertBigRedirectAlertAttributes(alertsRaised.get(0));
assertBigRedirectBigAlertAttributes(alertsRaised.get(0));
}

@Test
void givenRedirectHeadersWithSmallBodyButMultipleHrefsThenAlertRaised() {
// Given
msg.getResponseHeader().setStatusCode(HttpStatusCode.MOVED_PERMANENTLY);
msg.getResponseHeader().setHeader(HttpHeader.LOCATION, URI);
msg.setResponseBody(
"<html><a href=\""
+ URI.toString()
+ "\">Home</a>"
+ "<br>"
+ "<a href=\""
+ URI.toString()
+ "/admin\">Admin</a>"
+ "</html>");
// When
scanHttpResponseReceive(msg);
// Then
assertThat(alertsRaised.size(), is(1));
assertBigRedirectMultiAlertAttributes(alertsRaised.get(0), "2");
}

@Test
Expand Down Expand Up @@ -152,22 +174,47 @@ void shouldReturnExpectedMappings() {
is(equalTo(CommonAlertTag.WSTG_V42_INFO_05_CONTENT_LEAK.getValue())));
}

private static void assertBigRedirectAlertAttributes(Alert alert) {
@Test
void shouldHaveExpectedExampleAlerts() {
// Given / When
List<Alert> alerts = rule.getExampleAlerts();
// Then
assertThat(alerts.size(), is(equalTo(2)));
assertBigRedirectBigAlertAttributes(alerts.get(0));
assertBigRedirectMultiAlertAttributes(alerts.get(1), "3");
}

private static void assertBigRedirectBigAlertAttributes(Alert alert) {
assertThat(alert.getRisk(), is(Alert.RISK_LOW));
assertThat(alert.getConfidence(), is(Alert.CONFIDENCE_MEDIUM));
assertThat(alert.getName(), is(getLocalisedString("name")));
assertThat(alert.getDescription(), is(getLocalisedString("desc")));
assertThat(alert.getSolution(), is(getLocalisedString("soln")));
assertThat(alert.getReference(), is(getLocalisedString("refs")));
assertThat(alert.getOtherInfo(), is(getExpectedExtraInfo()));
assertThat(alert.getOtherInfo(), is(getExpectedBigExtraInfo()));
assertThat(alert.getCweId(), is(201));
assertThat(alert.getWascId(), is(13));
assertThat(alert.getAlertRef(), is("10044-1"));
}

private static String getExpectedBigExtraInfo() {
return getLocalisedString(
"extrainfo", URI.length(), URI, ALLOWABLE_BODY_SIZE, ALLOWABLE_BODY_SIZE + 1);
}

private static void assertBigRedirectMultiAlertAttributes(Alert alert, String count) {
assertThat(alert.getRisk(), is(Alert.RISK_LOW));
assertThat(alert.getConfidence(), is(Alert.CONFIDENCE_MEDIUM));
assertThat(alert.getName(), is(getLocalisedString("multi.name")));
assertThat(alert.getDescription(), is(getLocalisedString("multi.desc")));
assertThat(alert.getSolution(), is(getLocalisedString("soln")));
assertThat(alert.getOtherInfo(), is(getExpectedMultiExtraInfo(count)));
assertThat(alert.getCweId(), is(201));
assertThat(alert.getWascId(), is(13));
assertThat(alert.getUri(), is(URI));
assertThat(alert.getAlertRef(), is("10044-2"));
}

private static String getExpectedExtraInfo() {
int bodySize = ALLOWABLE_BODY_SIZE;
return getLocalisedString("extrainfo", URI.length(), URI, bodySize, bodySize + 1);
private static String getExpectedMultiExtraInfo(String count) {
return getLocalisedString("multi.extrainfo", count);
}

private static String getLocalisedString(String key, Object... params) {
Expand Down

0 comments on commit 1375f98

Please sign in to comment.