Skip to content

ascanrulesBeta: Add Example Alerts to InsecureHttpMethodScanRule #6871

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ascanrulesBeta: Add Example Alerts to InsecureHttpMethodScanRule #6871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed
- Depends on an updated version of the Common Library add-on.
- Reduced usage of error level logging.
- Insecure Http Method Scan Rule now includes example alert functionality for documentation generation purposes (Issue 6119) and alert references (Issue 7100).

## [62] - 2025-09-18
### Added
Expand Down
200 changes: 127 additions & 73 deletions ...ta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,24 +196,7 @@ public void scan() {
enabledMethodsSet.remove(
HttpRequestHeader
.DELETE); // We don't actually want to make a DELETE request
newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.detailed.name",
HttpRequestHeader.DELETE))
.setDescription(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.desc",
HttpRequestHeader.DELETE))
.setOtherInfo(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.extrainfo", allowedmethods))
.setSolution(
Constant.messages.getString("ascanbeta.insecurehttpmethod.soln"))
.setEvidence(HttpRequestHeader.DELETE)
.setMessage(optionsmsg)
.raise();
buildDeleteMethodAlert(allowedmethods).setMessage(optionsmsg).raise();
}

if (attackStrength == AttackStrength.HIGH || attackStrength == AttackStrength.INSANE) {
Expand Down Expand Up @@ -307,19 +290,8 @@ public void scan() {
if (raiseAlert) {
LOGGER.debug("Raising alert for Insecure HTTP Method");

newAlert()
.setRisk(riskLevel)
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.detailed.name",
insecureMethod))
.setDescription(description)
.setOtherInfo(extraInfo)
.setSolution(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.soln"))
.setEvidence(evidence)
buildInsecureHttpMethodAlert(
riskLevel, insecureMethod, description, extraInfo, evidence)
.setMessage(alertMessage)
.raise();
}
Expand Down Expand Up @@ -376,21 +348,8 @@ private void testTraceOrTrack(String method) throws Exception {

// if the response *body* from the TRACE request contains the cookie,we're in business :)
if (msg.getResponseBody().toString().contains(randomcookievalue)) {
newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.detailed.name", method))
.setDescription(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.trace.exploitable.desc", method))
.setUri(msg.getRequestHeader().getURI().toString())
.setOtherInfo(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.trace.exploitable.extrainfo",
randomcookievalue))
.setSolution(Constant.messages.getString("ascanbeta.insecurehttpmethod.soln"))
.setEvidence(randomcookievalue)
buildTraceCookieAlert(
randomcookievalue, method, msg.getRequestHeader().getURI().toString())
.setMessage(msg)
.raise();
}
Expand Down Expand Up @@ -495,24 +454,7 @@ private void handleConnectResponse(
Matcher m = thirdPartyContentPattern.matcher(response);
if (m.matches()) {
LOGGER.debug("Response matches expected third party pattern!");
newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.detailed.name",
HttpRequestHeader.CONNECT))
.setDescription(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.connect.exploitable.desc",
HttpRequestHeader.CONNECT))
.setOtherInfo(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.connect.exploitable.extrainfo",
thirdpartyHost))
.setSolution(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.soln"))
.setEvidence(response)
buildConnectResponseAlert(thirdpartyHost, response)
.setMessage(this.getBaseMsg())
.raise();
} else {
Expand Down Expand Up @@ -624,15 +566,8 @@ private void testHttpMethod(String httpMethod) throws Exception {
}
try {

newAlert()
.setRisk(riskLevel)
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.detailed.name", httpMethod))
.setDescription(exploitableDesc)
.setOtherInfo(exploitableExtraInfo)
.setEvidence(evidence)
buildHttpMethodAlert(
riskLevel, httpMethod, exploitableDesc, exploitableExtraInfo, evidence)
.setMessage(msg)
.raise();
} catch (Exception e) {
Expand All @@ -642,4 +577,123 @@ private void testHttpMethod(String httpMethod) throws Exception {
private static String randomAlphanumeric(int count) {
return RandomStringUtils.secure().nextAlphanumeric(count);
}

private AlertBuilder buildBaseAlert(String httpMethod) {
return newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.detailed.name", httpMethod))
.setSolution(Constant.messages.getString("ascanbeta.insecurehttpmethod.soln"));
}

private AlertBuilder buildDeleteMethodAlert(String allowedmethods) {
return buildBaseAlert(HttpRequestHeader.DELETE)
.setDescription(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.desc", HttpRequestHeader.DELETE))
.setOtherInfo(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.extrainfo", allowedmethods))
.setEvidence(HttpRequestHeader.DELETE)
.setAlertRef(getId() + "-1");
}

private AlertBuilder buildInsecureHttpMethodAlert(
int riskLevel,
String insecureMethod,
String description,
String extraInfo,
String evidence) {
return buildBaseAlert(insecureMethod)
.setRisk(riskLevel)
.setDescription(description)
.setOtherInfo(extraInfo)
.setEvidence(evidence)
.setAlertRef(getId() + "-2");
}

private AlertBuilder buildTraceCookieAlert(
String randomcookievalue, String method, String URI) {
return buildBaseAlert(method)
.setDescription(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.trace.exploitable.desc", method))
.setUri(URI)
.setOtherInfo(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.trace.exploitable.extrainfo",
randomcookievalue))
.setEvidence(randomcookievalue)
.setAlertRef(getId() + "-3");
}

private AlertBuilder buildConnectResponseAlert(String thirdpartyHost, String response) {
return buildBaseAlert(HttpRequestHeader.CONNECT)
.setDescription(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.connect.exploitable.desc",
HttpRequestHeader.CONNECT))
.setOtherInfo(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.connect.exploitable.extrainfo",
thirdpartyHost))
.setEvidence(response)
.setAlertRef(getId() + "-4");
}

private AlertBuilder buildHttpMethodAlert(
int riskLevel,
String httpMethod,
String exploitableDesc,
String exploitableExtraInfo,
String evidence) {
return buildBaseAlert(httpMethod)
.setRisk(riskLevel)
.setDescription(exploitableDesc)
.setOtherInfo(exploitableExtraInfo)
.setEvidence(evidence)
.setAlertRef(getId() + "-5");
}

@Override
public List<Alert> getExampleAlerts() {
return List.of(
buildDeleteMethodAlert("GET, POST, OPTIONS, HEAD, DELETE").build(),
buildInsecureHttpMethodAlert(
Alert.RISK_MEDIUM,
"PUT",
"PUT",
"GET, POST, HEAD, OPTIONS, PUT, DELETE",
"PUT")
.build(),
buildTraceCookieAlert(
"aB3kL9zT2vQw8Xy1RmNp6sGh4Ud7Fc0ZjK9yX2mP",
"TRACE",
"http://example.com/")
.build(),
buildConnectResponseAlert(
"www.google.com",
"HTTP/1.1 200 OK\n"
+ "Date: Sat, 01 Nov 2025 19:33:10 GMT\n"
+ "Content-Type: text/html; charset=ISO-8859-1\n"
+ "Server: gws\n"
+ "Connection: close")
.build(),
buildHttpMethodAlert(
Copy link
Member

@thc202 thc202 Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needs to be extracted to its own method, it's also missing the webdav which IMO should have its own ref (i.e the one in line 569 should be replaced with two build/raise calls in lines 545 and 569, having all the data already filled in).

Copy link
Author

@Brandosp Brandosp Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, not exactly sure what you mean by this. I began working on the code based off this comment, and I added to the buildHttpMethodAlert method to check whether or not the httpMethod is part of WEBDAV or not to give webdav its own alert ref

String alertRef = getId() + "-5"; if (WEBDAV_METHODS.contains(httpMethod)) { alertRef = getId() + "-6"; }
You mentioned adding two build/raise calls in line 545 and 569 which I assume this means I should create a buildAlert for specifically WEBDAV but I found this confusing as the main logic filters the data for whether the httpmethod is part of WEBDAV or not making it so the build call on 569 would create distinct alerts, so I imagine my assumption is wrong?

Additionally, when u mention extracting to its own method, do u mean creating a buildExampleAlert helper which would just use the code I have in the getExampleAlerts for the HttpMethodAlert, and then simply calling that helper function to make the code more structured?

Copy link
Member

@thc202 thc202 Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two issues:

  1. The getExampleAlerts() should not be duplicating what the main code is doing, both should call a common method. If tomorrow we change the description/other info of the lines 553 and 559, this example alert will keep returning the old/different description/other info. We should not need to manually keep main code and examples in sync. Also, note this issue is already happening with buildInsecureHttpMethodAlert, the example alert has the wrong description/other info.
  2. The webdav case of line 545 is effectively a different alert, note the use of different description and why (IMO) it should have its own ref.

Alert.RISK_MEDIUM,
"PUT",
Constant.messages.getString(
"ascanbeta.insecurehttpmethod."
+ "put"
+ ".exploitable.desc",
"PUT"),
Constant.messages.getString(
"ascanbeta.insecurehttpmethod."
+ "put"
+ ".exploitable.extrainfo"),
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.insecure", 201))
.build());
}
}
10 changes: 10 additions & 0 deletions ...est/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRuleUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@
import fi.iki.elonen.NanoHTTPD.IHTTPSession;
import fi.iki.elonen.NanoHTTPD.Method;
import fi.iki.elonen.NanoHTTPD.Response;
import java.util.List;
import java.util.Map;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.junit.jupiter.params.provider.ValueSource;
import org.parosproxy.paros.core.scanner.Alert;
import org.parosproxy.paros.network.HttpMessage;
import org.zaproxy.addon.commonlib.CommonAlertTag;
import org.zaproxy.addon.commonlib.PolicyTag;
Expand Down Expand Up @@ -139,4 +141,12 @@ void shouldReturnExpectedMappings() {
tags.get(CommonAlertTag.WSTG_V42_CONF_06_HTTP_METHODS.getTag()),
is(equalTo(CommonAlertTag.WSTG_V42_CONF_06_HTTP_METHODS.getValue())));
}

@Test
void shouldHaveExpectedExampleAlert() {
// Given / When
List<Alert> alerts = rule.getExampleAlerts();
// Then
assertThat(alerts.size(), is(equalTo(5)));
}
}