diff --git a/addOns/ascanrulesBeta/CHANGELOG.md b/addOns/ascanrulesBeta/CHANGELOG.md index 055f1767e5..c5496d64bd 100644 --- a/addOns/ascanrulesBeta/CHANGELOG.md +++ b/addOns/ascanrulesBeta/CHANGELOG.md @@ -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 +- Maintenance changes. +- Updated the Insecure HTTP Method rule to raise alerts at LOW confidence when the Alert Threshold is LOW. Corrected the identification of HTTP methods/verbs which may result in more alerts being raised (regardless of Alert Threshold). ## [62] - 2025-09-18 ### Added diff --git a/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRule.java b/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRule.java index 0fed3819a5..8b75d10f9a 100644 --- a/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRule.java +++ b/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRule.java @@ -98,6 +98,19 @@ public class InsecureHttpMethodScanRule extends AbstractAppPlugin INSECURE_METHODS.addAll(WEBDAV_METHODS); } + private static final Set METHODS_TO_SKIP = + Set.of(HttpRequestHeader.GET, HttpRequestHeader.POST, HttpRequestHeader.HEAD); + + /* + * Build a list with status codes which indicate that this HTTP method is + * enabled but we are not allowed to use it + */ + private static final List ENABLED_STATUS_CODES = + List.of( + HttpStatusCode.UNAUTHORIZED, + HttpStatusCode.PAYMENT_REQUIRED, + HttpStatusCode.FORBIDDEN); + private static final Map ALERT_TAGS; static { @@ -150,7 +163,7 @@ public void scan() { String thirdpartyHost = "www.google.com"; int thirdpartyPort = 80; Pattern thirdPartyContentPattern = - Pattern.compile("", Pattern.CASE_INSENSITIVE); + Pattern.compile("", Pattern.CASE_INSENSITIVE); // send an OPTIONS message, and see what the server reports. Do // not try any methods not listed in those results. @@ -191,7 +204,12 @@ public void scan() { // Convert the list to a set so that we have a unique list Set enabledMethodsSet = new HashSet<>( - Arrays.asList(allowedmethods.toUpperCase(Locale.ROOT).split(","))); + Arrays.stream(allowedmethods.toUpperCase(Locale.ROOT).split(",")) + .map(String::trim) // strip off any leading spaces (it happens!) + .toList()); + + // Remove methods we aren't concerned about + enabledMethodsSet.removeAll(METHODS_TO_SKIP); if (enabledMethodsSet.contains(HttpRequestHeader.DELETE)) { enabledMethodsSet.remove( HttpRequestHeader @@ -245,12 +263,6 @@ public void scan() { // rely on the OPTIONS METHOD, but potentially verify the // results, depending on the Threshold. for (String enabledmethod : enabledMethodsSet) { - enabledmethod = - enabledmethod.trim(); // strip off any leading spaces (it happens!) - if (enabledmethod.isEmpty()) { - continue; - } - LOGGER.debug( "The following enabled method is being checked: '{}'", enabledmethod); String insecureMethod = enabledmethod; @@ -306,10 +318,9 @@ public void scan() { if (raiseAlert) { LOGGER.debug("Raising alert for Insecure HTTP Method"); - newAlert() .setRisk(riskLevel) - .setConfidence(Alert.CONFIDENCE_MEDIUM) + .setConfidence(Alert.CONFIDENCE_LOW) .setName( Constant.messages.getString( "ascanbeta.insecurehttpmethod.detailed.name", @@ -565,16 +576,6 @@ private void testHttpMethod(String httpMethod) throws Exception { } final int responseCode = msg.getResponseHeader().getStatusCode(); - String evidence = ""; - - /* - * Build a list with status code which indicate that this HTTP method is - * enabled but we are not allowed to use it - */ - final ArrayList enabledStatusCodes = new ArrayList<>(); - enabledStatusCodes.add(HttpStatusCode.UNAUTHORIZED); - enabledStatusCodes.add(HttpStatusCode.PAYMENT_REQUIRED); - enabledStatusCodes.add(HttpStatusCode.FORBIDDEN); LOGGER.debug("Request Method: {}", httpMethod); LOGGER.debug("Response Code: {}", responseCode); @@ -584,19 +585,15 @@ private void testHttpMethod(String httpMethod) throws Exception { return; } - if (isPage200(msg) || responseCode == HttpStatusCode.CREATED) { - evidence = - Constant.messages.getString( - "ascanbeta.insecurehttpmethod.insecure", responseCode); - } else if (enabledStatusCodes.contains(responseCode)) { - evidence = + boolean isEnabledStatus = ENABLED_STATUS_CODES.contains(responseCode); + String furtherInfo = ""; + + if (isEnabledStatus) { + furtherInfo = Constant.messages.getString( "ascanbeta.insecurehttpmethod.potentiallyinsecure", responseCode); - } else { - return; } - int riskLevel; String exploitableDesc; String exploitableExtraInfo; if (WEBDAV_METHODS.contains(httpMethod)) { @@ -606,7 +603,6 @@ private void testHttpMethod(String httpMethod) throws Exception { exploitableExtraInfo = Constant.messages.getString( "ascanbeta.insecurehttpmethod.webdav.exploitable.extrainfo"); - riskLevel = Alert.RISK_INFO; } else { exploitableDesc = Constant.messages.getString( @@ -619,24 +615,24 @@ private void testHttpMethod(String httpMethod) throws Exception { "ascanbeta.insecurehttpmethod." + httpMethod.toLowerCase() + ".exploitable.extrainfo"); - - riskLevel = Alert.RISK_MEDIUM; } - try { - newAlert() - .setRisk(riskLevel) - .setConfidence(Alert.CONFIDENCE_MEDIUM) - .setName( - Constant.messages.getString( - "ascanbeta.insecurehttpmethod.detailed.name", httpMethod)) - .setDescription(exploitableDesc) - .setOtherInfo(exploitableExtraInfo) - .setEvidence(evidence) - .setMessage(msg) - .raise(); - } catch (Exception e) { - } + exploitableExtraInfo = + StringUtils.isNotEmpty(furtherInfo) + ? furtherInfo + "\n\n" + exploitableExtraInfo + : exploitableExtraInfo; + + newAlert() + .setRisk(Alert.RISK_MEDIUM) + .setConfidence(isEnabledStatus ? Alert.CONFIDENCE_LOW : Alert.CONFIDENCE_MEDIUM) + .setName( + Constant.messages.getString( + "ascanbeta.insecurehttpmethod.detailed.name", httpMethod)) + .setDescription(exploitableDesc) + .setOtherInfo(exploitableExtraInfo) + .setEvidence(String.valueOf(responseCode)) + .setMessage(msg) + .raise(); } private static String randomAlphanumeric(int count) { diff --git a/addOns/ascanrulesBeta/src/main/resources/org/zaproxy/zap/extension/ascanrulesBeta/resources/Messages.properties b/addOns/ascanrulesBeta/src/main/resources/org/zaproxy/zap/extension/ascanrulesBeta/resources/Messages.properties index 87be5b9511..674cb3ca0c 100644 --- a/addOns/ascanrulesBeta/src/main/resources/org/zaproxy/zap/extension/ascanrulesBeta/resources/Messages.properties +++ b/addOns/ascanrulesBeta/src/main/resources/org/zaproxy/zap/extension/ascanrulesBeta/resources/Messages.properties @@ -93,13 +93,12 @@ ascanbeta.insecurehttpmethod.delete.exploitable.extrainfo = See the discussion o ascanbeta.insecurehttpmethod.desc = The insecure HTTP method [{0}] is enabled on the web server for this resource. Depending on the web server configuration, and the underlying implementation responsible for serving the resource, this might or might not be exploitable. The TRACK and TRACE methods may be used by an attacker, to gain access to the authorisation token/session cookie of an application user, even if the session cookie is protected using the 'HttpOnly' flag. For the attack to be successful, the application user must typically be using an older web browser, or a web browser which has a Same Origin Policy (SOP) bypass vulnerability. The 'CONNECT' method can be used by a web client to create an HTTP tunnel to third party websites or services. ascanbeta.insecurehttpmethod.detailed.name = Insecure HTTP Method - {0} ascanbeta.insecurehttpmethod.extrainfo = The OPTIONS method disclosed the following enabled HTTP methods for this resource: [{0}] -ascanbeta.insecurehttpmethod.insecure = response code {0} for insecure HTTP METHOD ascanbeta.insecurehttpmethod.name = Insecure HTTP Method ascanbeta.insecurehttpmethod.options.exploitable.desc = This is a diagnostic method and should never be turned on in production mode. ascanbeta.insecurehttpmethod.options.exploitable.extrainfo = See the discussion on stackexchange: https://security.stackexchange.com/questions/21413/how-to-exploit-http-methods ascanbeta.insecurehttpmethod.patch.exploitable.desc = This method is now most commonly used in REST services, PATCH is used for **modify** capabilities. The PATCH request only needs to contain the changes to the resource, not the complete resource. ascanbeta.insecurehttpmethod.patch.exploitable.extrainfo = See the discussion on stackexchange: https://security.stackexchange.com/questions/21413/how-to-exploit-http-methods, for understanding REST operations see https://www.restapitutorial.com/lessons/httpmethods.html -ascanbeta.insecurehttpmethod.potentiallyinsecure = response code {0} for potentially insecure HTTP METHOD +ascanbeta.insecurehttpmethod.potentiallyinsecure = Received response code {0} for potentially insecure HTTP method. This suggests it is enabled or supported but some control prevented us from actually using it. ascanbeta.insecurehttpmethod.put.exploitable.desc = This method was originally intended for file management operations. It is now most commonly used in REST services, PUT is most-often utilized for **update** capabilities, PUT-ing to a known resource URI with the request body containing the newly-updated representation of the original resource. ascanbeta.insecurehttpmethod.put.exploitable.extrainfo = See the discussion on stackexchange: https://security.stackexchange.com/questions/21413/how-to-exploit-http-methods, for understanding REST operations see https://www.restapitutorial.com/lessons/httpmethods.html ascanbeta.insecurehttpmethod.soln = Disable insecure methods such as TRACK, TRACE, and CONNECT on the web server, and ensure that the underlying service implementation does not support insecure methods. diff --git a/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRuleUnitTest.java b/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRuleUnitTest.java index 74ffad4376..e06a1f64c9 100644 --- a/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRuleUnitTest.java +++ b/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/InsecureHttpMethodScanRuleUnitTest.java @@ -21,17 +21,23 @@ import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import fi.iki.elonen.NanoHTTPD.IHTTPSession; import fi.iki.elonen.NanoHTTPD.Method; import fi.iki.elonen.NanoHTTPD.Response; +import fi.iki.elonen.NanoHTTPD.Response.Status; +import java.util.List; import java.util.Map; +import java.util.stream.Stream; 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.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import org.parosproxy.paros.core.scanner.Alert; +import org.parosproxy.paros.core.scanner.Plugin.AlertThreshold; import org.parosproxy.paros.network.HttpMessage; import org.zaproxy.addon.commonlib.CommonAlertTag; import org.zaproxy.addon.commonlib.PolicyTag; @@ -39,72 +45,182 @@ class InsecureHttpMethodScanRuleUnitTest extends ActiveScannerTest { + private static final String PATH = "/test/"; + private static final List PUT_PATCH = List.of("PUT", "PATCH"); + private static final List API_CONTENT_TYPES = List.of("json", "xml"); + private static final List THRESHOLDS = + List.of(AlertThreshold.HIGH, AlertThreshold.MEDIUM, AlertThreshold.LOW); + @Override protected InsecureHttpMethodScanRule createScanner() { return new InsecureHttpMethodScanRule(); } - private static class AllowedPutPatchNanoServerHandler extends NanoServerHandler { + @Override + protected boolean isIgnoreAlertsRaisedInSendReasonableNumberOfMessages() { + return true; + } - String contentType; - String method; + private static Stream providePutPatchApiCombinations(AlertThreshold threshold) { + return PUT_PATCH.stream() + .flatMap( + method -> + API_CONTENT_TYPES.stream() + .map(ct -> Arguments.of(method, ct, threshold))); + } - AllowedPutPatchNanoServerHandler(String method, String contentType, String path) { - super(path); - this.method = method; - this.contentType = contentType; - } + private static Stream providePutPatchApiCombosLowThreshold() { + return providePutPatchApiCombinations(AlertThreshold.LOW); + } - @Override - protected Response serve(IHTTPSession session) { - Response response = newFixedLengthResponse(""); + private static Stream providePutPatchApiCombosMediumAndHighThreshold() { + return Stream.concat( + providePutPatchApiCombinations(AlertThreshold.MEDIUM), + providePutPatchApiCombinations(AlertThreshold.HIGH)); + } - if (session.getMethod().equals(Method.OPTIONS)) { - response.addHeader("Allow", method); - return response; - } + @ParameterizedTest + @MethodSource("providePutPatchApiCombosMediumAndHighThreshold") + void shouldNotRaiseAlertsForPutOrPatchMethodsIfReturnJsonOrXmlAtHighOrMediumThreshold( + String method, String contentType, AlertThreshold threshold) throws Exception { + // Given + HttpMessage message = getHttpMessage(PATH); + nano.addHandler(new OkayPutPatchNanoServerHandler(method, contentType)); - if (session.getMethod().equals(Method.PUT) - || session.getMethod().equals(Method.PATCH)) { - response.setMimeType(contentType); - return response; - } - consumeBody(session); - return response; - } + rule.init(message, parent); + rule.setAlertThreshold(threshold); + // When + rule.scan(); + // Then + assertThat(alertsRaised, is(empty())); } @ParameterizedTest - @CsvSource({"PUT, json", "PUT, xml", "PATCH, json", "PATCH, xml"}) - void shouldRaiseNoAlertsForPutOrPatchMethodsIfReturnJsonOrXml(String method, String contentType) - throws Exception { + @MethodSource("providePutPatchApiCombosLowThreshold") + void shouldRaiseAlertsForPutOrPatchMethodsIfReturnJsonOrXmlAtLowThreshold( + String method, String contentType, AlertThreshold threshold) throws Exception { // Given - String path = "/shouldRaiseNoAlertsForPutOrPatchMethodsIfReturnJsonOrXml/"; - HttpMessage message = getHttpMessage(path); - nano.addHandler(new AllowedPutPatchNanoServerHandler(method, contentType, path)); + HttpMessage message = getHttpMessage(PATH); + nano.addHandler(new OkayPutPatchNanoServerHandler(method, contentType)); rule.init(message, parent); + rule.setAlertThreshold(threshold); // When rule.scan(); // Then - assertThat(alertsRaised.size(), equalTo(0)); + assertThat(alertsRaised.size(), equalTo(1)); + Alert alert = alertsRaised.get(0); + assertThat(alert.getRisk(), is(equalTo(Alert.RISK_MEDIUM))); + assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW))); + assertThat( + alert.getOtherInfo(), + is( + "The OPTIONS method disclosed the following enabled HTTP methods for this resource: [%s]" + .formatted(method))); + System.out.println(alert.getRisk()); + System.out.println(alert.getConfidence()); + System.out.println(alert.getOtherInfo()); + } + + private static Stream providePutPatchHtmlCombinations() { + return PUT_PATCH.stream() + .flatMap( + method -> + THRESHOLDS.stream() + .map(threshold -> Arguments.of(method, "html", threshold))); + } + + @ParameterizedTest + @MethodSource("providePutPatchHtmlCombinations") + void shouldRaiseAlertForPutOrPatchMethodsIfNotReturnJsonOrXmlAnyThreshold( + String method, String contentType, AlertThreshold threshold) throws Exception { + // Given + HttpMessage message = getHttpMessage(PATH); + nano.addHandler(new OkayPutPatchNanoServerHandler(method, contentType)); + + rule.init(message, parent); + rule.setAlertThreshold(threshold); + // When + rule.scan(); + // Then + assertThat(alertsRaised.size(), equalTo(1)); + assertThat(alertsRaised.get(0).getName(), equalTo("Insecure HTTP Method - " + method)); + Alert alert = alertsRaised.get(0); + assertThat(alert.getRisk(), is(equalTo(Alert.RISK_MEDIUM))); + // Whenever threshold is LOW confidence should be LOW as the OPTIONS response method list is + // trusted + assertThat( + alert.getConfidence(), + is( + equalTo( + AlertThreshold.LOW.equals(rule.getAlertThreshold()) + ? Alert.CONFIDENCE_LOW + : Alert.CONFIDENCE_MEDIUM))); + assertOkayOtherInfoByThreshold(method, threshold, alert); + } + + private static void assertOkayOtherInfoByThreshold( + String method, AlertThreshold threshold, Alert alert) { + switch (threshold) { + case LOW -> { + assertThat( + alert.getOtherInfo(), + is( + equalTo( + "The OPTIONS method disclosed the following enabled HTTP methods for this resource: [%s]" + .formatted(method)))); + } + default -> { // MEDIUM or HIGH + assertThat( + alert.getOtherInfo(), + is( + equalTo( + "See the discussion on stackexchange: https://security.stackexchange.com/questions/21413/how-to-exploit-http-methods, for understanding REST operations see https://www.restapitutorial.com/lessons/httpmethods.html"))); + } + } } @ParameterizedTest - @ValueSource(strings = {"PUT", "PATCH"}) - void shouldRaiseAlertForPutOrPatchMethodsIfNotReturnJsonOrXml(String method) throws Exception { + @MethodSource("providePutPatchHtmlCombinations") + void shouldRaiseAlertForPutOrPatchMethodsIfNotReturnJsonOrXmlAnyThresholdForbiddenStatus( + String method, String contentType, AlertThreshold threshold) throws Exception { // Given - String path = "/shouldRaiseAlertForPutOrPatchMethodsIfNotReturnJsonOrXml/"; - String contentType = "html"; - HttpMessage message = getHttpMessage(path); - nano.addHandler(new AllowedPutPatchNanoServerHandler(method, contentType, path)); + HttpMessage message = getHttpMessage(PATH); + nano.addHandler(new ForbiddenPutPatchNanoServerHandler(method, contentType)); rule.init(message, parent); + rule.setAlertThreshold(threshold); // When rule.scan(); // Then assertThat(alertsRaised.size(), equalTo(1)); assertThat(alertsRaised.get(0).getName(), equalTo("Insecure HTTP Method - " + method)); + + Alert alert = alertsRaised.get(0); + assertThat(alert.getRisk(), is(equalTo(Alert.RISK_MEDIUM))); + assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW))); + assertForbiddenOtherInfoByThreshold(method, threshold, alert); + } + + private static void assertForbiddenOtherInfoByThreshold( + String method, AlertThreshold threshold, Alert alert) { + switch (threshold) { + case LOW -> { + assertThat( + alert.getOtherInfo(), + is( + equalTo( + "The OPTIONS method disclosed the following enabled HTTP methods for this resource: [%s]" + .formatted(method)))); + } + default -> { // MEDIUM or HIGH + assertThat( + alert.getOtherInfo(), + is( + equalTo( + "Received response code 403 for potentially insecure HTTP method. This suggests it is enabled or supported but some control prevented us from actually using it.\n\nSee the discussion on stackexchange: https://security.stackexchange.com/questions/21413/how-to-exploit-http-methods, for understanding REST operations see https://www.restapitutorial.com/lessons/httpmethods.html"))); + } + } } @Test @@ -139,4 +255,51 @@ void shouldReturnExpectedMappings() { tags.get(CommonAlertTag.WSTG_V42_CONF_06_HTTP_METHODS.getTag()), is(equalTo(CommonAlertTag.WSTG_V42_CONF_06_HTTP_METHODS.getValue()))); } + + private static class OkayPutPatchNanoServerHandler + extends SpecificStatusPutPatchNanoServerHandler { + OkayPutPatchNanoServerHandler(String method, String contentType) { + super(method, contentType, Status.OK); + } + } + + private static class ForbiddenPutPatchNanoServerHandler + extends SpecificStatusPutPatchNanoServerHandler { + ForbiddenPutPatchNanoServerHandler(String method, String contentType) { + super(method, contentType, Status.FORBIDDEN); + } + } + + private static class SpecificStatusPutPatchNanoServerHandler extends NanoServerHandler { + + String contentType; + String method; + Status status; + + SpecificStatusPutPatchNanoServerHandler(String method, String contentType, Status status) { + super(PATH); + this.method = method; + this.contentType = contentType; + this.status = status; + } + + @Override + protected Response serve(IHTTPSession session) { + Response response = newFixedLengthResponse(""); + + if (session.getMethod().equals(Method.OPTIONS)) { + response.addHeader("Allow", method); + return response; + } + + if (session.getMethod().equals(Method.PUT) + || session.getMethod().equals(Method.PATCH)) { + response.setMimeType(contentType); + response.setStatus(status); + return response; + } + consumeBody(session); + return response; + } + } }