From 1151ac23559707b7431fb2414d9c7e497aefedc3 Mon Sep 17 00:00:00 2001 From: kingthorin Date: Wed, 4 Sep 2024 12:04:49 -0400 Subject: [PATCH] ascanrules: More example alerts - Scan Rules > Added example alert handling. - Unit Tests > Added tests for the example alerts and references. - CHANGELOG > Added change note. Signed-off-by: kingthorin --- addOns/ascanrules/CHANGELOG.md | 5 ++- .../ascanrules/XpathInjectionScanRule.java | 22 ++++++++---- .../ascanrules/XsltInjectionScanRule.java | 35 ++++++++++--------- .../XpathInjectionScanRuleUnitTest.java | 20 +++++++++++ .../XsltInjectionScanRuleUnitTest.java | 18 ++++++++++ 5 files changed, 76 insertions(+), 24 deletions(-) diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md index f9f4b850efd..d2f13b36a8e 100644 --- a/addOns/ascanrules/CHANGELOG.md +++ b/addOns/ascanrules/CHANGELOG.md @@ -6,7 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## Unreleased ### Changed - Maintenance changes. -- The Spring Actuator Scan Rule now includes example alert functionality for documentation generation purposes (Issue 6119). +- The following scan rules now include example alert functionality for documentation generation purposes (Issue 6119): + - Spring Actuator + - XSLT Injection + - XPath Injection ## [67] - 2024-07-22 diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRule.java index 4d6516373ff..4243adbb93e 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRule.java @@ -20,6 +20,7 @@ package org.zaproxy.zap.extension.ascanrules; import java.io.IOException; +import java.util.List; import java.util.Map; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; @@ -199,13 +200,7 @@ public void scan(HttpMessage msg, String paramName, String value) { paramName, evilPayload); - newAlert() - .setConfidence(Alert.CONFIDENCE_HIGH) - .setParam(paramName) - .setAttack(evilPayload) - .setEvidence(errorString) - .setMessage(msg) - .raise(); + createAlert(paramName, evilPayload, errorString).setMessage(msg).raise(); // All done. No need to look for vulnerabilities on subsequent // parameters on the same request (to reduce performance impact) @@ -232,4 +227,17 @@ public void scan(HttpMessage msg, String paramName, String value) { } } } + + private AlertBuilder createAlert(String param, String payload, String evidence) { + return newAlert() + .setConfidence(Alert.CONFIDENCE_HIGH) + .setParam(param) + .setAttack(payload) + .setEvidence(evidence); + } + + @Override + public List getExampleAlerts() { + return List.of(createAlert("foo", XPATH_PAYLOADS[0], XPATH_ERRORS[0]).build()); + } } diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRule.java index 16719375142..292198ec54b 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRule.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.Arrays; import java.util.EnumMap; +import java.util.List; import java.util.Map; import java.util.function.Predicate; import org.apache.commons.httpclient.URIException; @@ -184,12 +185,9 @@ private boolean tryInjection(String param, XSLTInjectionType checkType) { } if (raiseAlert) { - raiseAlert( - msg, - param, - payload, - evidence, - checkType.getResourceIdentifier()); + createAlert(param, payload, evidence, checkType.getResourceIdentifier()) + .setMessage(msg) + .raise(); return true; } } @@ -242,20 +240,14 @@ private static String getOtherInfo(String resourceIdentifier, String param) { MESSAGE_PREFIX + resourceIdentifier + ".otherinfo", param); } - private void raiseAlert( - HttpMessage msg, - String param, - String attack, - String evidence, - String resourceIdentifier) { - newAlert() + private AlertBuilder createAlert( + String param, String attack, String evidence, String resourceIdentifier) { + return newAlert() .setConfidence(Alert.CONFIDENCE_MEDIUM) .setParam(param) .setAttack(attack) .setOtherInfo(getOtherInfo(resourceIdentifier, evidence)) - .setEvidence(evidence) - .setMessage(msg) - .raise(); + .setEvidence(evidence); } @Override @@ -307,4 +299,15 @@ public Map getAlertTags() { public int getRisk() { return Alert.RISK_MEDIUM; } + + @Override + public List getExampleAlerts() { + return List.of( + createAlert( + "foo", + XSLTInjectionType.ERROR.getPayloads(null)[0], + XSLTInjectionType.ERROR.getEvidences()[1], + XSLTInjectionType.ERROR.getResourceIdentifier()) + .build()); + } } diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRuleUnitTest.java index 4ceca301f8e..d6bbd335020 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRuleUnitTest.java @@ -21,10 +21,13 @@ 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 java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; +import org.parosproxy.paros.core.scanner.Alert; import org.zaproxy.addon.commonlib.CommonAlertTag; class XpathInjectionScanRuleUnitTest extends ActiveScannerTest { @@ -63,4 +66,21 @@ void shouldReturnExpectedMappings() { tags.get(CommonAlertTag.WSTG_V42_INPV_09_XPATH.getTag()), is(equalTo(CommonAlertTag.WSTG_V42_INPV_09_XPATH.getValue()))); } + + @Test + void shouldHaveExpectedExampleAlert() { + // Given / When + List alerts = rule.getExampleAlerts(); + // Then + assertThat(alerts, hasSize(1)); + Alert alert = alerts.get(0); + assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_HIGH))); + assertThat(alert.getAlertRef(), is(equalTo("90021"))); + } + + @Test + @Override + public void shouldHaveValidReferences() { + super.shouldHaveValidReferences(); + } } diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRuleUnitTest.java index 5a21861e025..32c933d55b4 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/XsltInjectionScanRuleUnitTest.java @@ -25,6 +25,7 @@ import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; +import java.util.List; import java.util.Map; import org.junit.jupiter.api.Test; import org.parosproxy.paros.core.scanner.Alert; @@ -163,4 +164,21 @@ void shouldReturnExpectedMappings() { tags.get(CommonAlertTag.OWASP_2017_A01_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2017_A01_INJECTION.getValue()))); } + + @Test + void shouldHaveExpectedExampleAlert() { + // Given / When + List alerts = rule.getExampleAlerts(); + // Then + assertThat(alerts, hasSize(1)); + Alert alert = alerts.get(0); + assertThat(alert.getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM))); + assertThat(alert.getAlertRef(), is(equalTo("90017"))); + } + + @Test + @Override + public void shouldHaveValidReferences() { + super.shouldHaveValidReferences(); + } }