From c6000a246207743311ebfdbdbc26f48d5cc61c09 Mon Sep 17 00:00:00 2001 From: kingthorin Date: Tue, 17 Dec 2024 06:22:39 -0500 Subject: [PATCH] various: Add TEST_TIMING alert tag Update: CHANGELOGs, scan rules, unittests. Signed-off-by: kingthorin --- addOns/ascanrules/CHANGELOG.md | 3 + .../ascanrules/CommandInjectionScanRule.java | 35 ++++++-- .../SqlInjectionHypersonicScanRule.java | 3 +- .../ascanrules/SqlInjectionMsSqlScanRule.java | 3 +- .../ascanrules/SqlInjectionMySqlScanRule.java | 3 +- .../SqlInjectionOracleScanRule.java | 3 +- .../SqlInjectionPostgreScanRule.java | 3 +- .../SqlInjectionSqLiteScanRule.java | 15 +++- .../ascanrules/SstiBlindScanRule.java | 3 +- .../CommandInjectionScanRuleUnitTest.java | 5 +- ...qlInjectionHypersonicScanRuleUnitTest.java | 3 +- .../SqlInjectionMsSqlScanRuleUnitTest.java | 3 +- .../SqlInjectionMySqlScanRuleUnitTest.java | 3 +- .../SqlInjectionOracleScanRuleUnitTest.java | 3 +- .../SqlInjectionPostgreScanRuleUnitTest.java | 3 +- .../SqlInjectionSQLiteScanRuleUnitTest.java | 7 +- .../ascanrules/SstiBlindScanRuleUnitTest.java | 3 +- addOns/ascanrulesBeta/CHANGELOG.md | 1 + .../ascanrulesBeta/ShellShockScanRule.java | 14 +++- .../ShellShockScanRuleUnitTest.java | 7 +- addOns/sqliplugin/CHANGELOG.md | 1 + addOns/sqliplugin/sqliplugin.gradle.kts | 2 + .../sqliplugin/SQLInjectionScanRule.java | 41 +++++++--- .../sqliplugin/ActiveScannerTest.java | 31 +++++++ .../SQLInjectionScanRuleUnitTest.java | 80 +++++++++++++++++++ 25 files changed, 247 insertions(+), 31 deletions(-) create mode 100644 addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/ActiveScannerTest.java create mode 100644 addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRuleUnitTest.java diff --git a/addOns/ascanrules/CHANGELOG.md b/addOns/ascanrules/CHANGELOG.md index d3d43204f8a..f4691f4ce49 100644 --- a/addOns/ascanrules/CHANGELOG.md +++ b/addOns/ascanrules/CHANGELOG.md @@ -10,6 +10,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - External Redirect scan rule to regenerate anti CSRF tokens. +### Changed +- Scan rules which execute time based attacks now include the "TEST_TIMING" alert tag. + ## [70] - 2025-01-09 ### Changed - Update minimum ZAP version to 2.16.0. diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java index 0896ba48d4d..7f85e8396bd 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java @@ -98,7 +98,8 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ)); + CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ, + CommonAlertTag.TEST_TIMING)); alertTags.put(PolicyTag.API.getTag(), ""); alertTags.put(PolicyTag.DEV_CICD.getTag(), ""); alertTags.put(PolicyTag.DEV_STD.getTag(), ""); @@ -367,6 +368,15 @@ public Map getAlertTags() { return ALERT_TAGS; } + private Map getNeededAlertTags(TestType type) { + Map alertTags = new HashMap<>(); + alertTags.putAll(getAlertTags()); + if (TestType.FEEDBACK.equals(type)) { + alertTags.remove(CommonAlertTag.TEST_TIMING.getTag()); + } + return alertTags; + } + @Override public int getCweId() { return 78; @@ -584,7 +594,14 @@ private boolean testCommandInjection( paramValue); String otherInfo = getOtherInfo(TestType.FEEDBACK, paramValue); - buildAlert(paramName, paramValue, matcher.group(), otherInfo, msg).raise(); + buildAlert( + paramName, + paramValue, + matcher.group(), + otherInfo, + TestType.FEEDBACK, + msg) + .raise(); // All done. No need to look for vulnerabilities on subsequent // payloads on the same request (to reduce performance impact) @@ -670,7 +687,8 @@ private boolean testCommandInjection( String otherInfo = getOtherInfo(TestType.TIME, paramValue); // just attach this alert to the last sent message - buildAlert(paramName, paramValue, "", otherInfo, message.get()).raise(); + buildAlert(paramName, paramValue, "", otherInfo, TestType.TIME, message.get()) + .raise(); // All done. No need to look for vulnerabilities on subsequent // payloads on the same request (to reduce performance impact) @@ -719,14 +737,20 @@ private static String insertUninitVar(String cmd) { } private AlertBuilder buildAlert( - String param, String attack, String evidence, String otherInfo, HttpMessage msg) { + String param, + String attack, + String evidence, + String otherInfo, + TestType type, + HttpMessage msg) { return newAlert() .setConfidence(Alert.CONFIDENCE_MEDIUM) .setParam(param) .setAttack(attack) .setEvidence(evidence) .setMessage(msg) - .setOtherInfo(otherInfo); + .setOtherInfo(otherInfo) + .setTags(getNeededAlertTags(type)); } @Override @@ -737,6 +761,7 @@ public List getExampleAlerts() { "a;cat /etc/passwd ", "root:x:0:0", getOtherInfo(TestType.FEEDBACK, "a;cat /etc/passwd "), + TestType.FEEDBACK, null) .build()); } diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRule.java index 2b4bffa24b9..b808d3f3535 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRule.java @@ -199,7 +199,8 @@ public class SqlInjectionHypersonicScanRule extends AbstractAppParamPlugin CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_05_SQLI)); + CommonAlertTag.WSTG_V42_INPV_05_SQLI, + CommonAlertTag.TEST_TIMING)); alertTags.put(PolicyTag.DEV_FULL.getTag(), ""); alertTags.put(PolicyTag.QA_STD.getTag(), ""); alertTags.put(PolicyTag.QA_FULL.getTag(), ""); diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java index f1e1b494265..594767042a9 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java @@ -144,7 +144,8 @@ public class SqlInjectionMsSqlScanRule extends AbstractAppParamPlugin CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_05_SQLI)); + CommonAlertTag.WSTG_V42_INPV_05_SQLI, + CommonAlertTag.TEST_TIMING)); alertTags.put(PolicyTag.DEV_FULL.getTag(), ""); alertTags.put(PolicyTag.QA_STD.getTag(), ""); alertTags.put(PolicyTag.QA_FULL.getTag(), ""); diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRule.java index 16a96d8dc1d..a74f1319858 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRule.java @@ -218,7 +218,8 @@ public class SqlInjectionMySqlScanRule extends AbstractAppParamPlugin CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_05_SQLI)); + CommonAlertTag.WSTG_V42_INPV_05_SQLI, + CommonAlertTag.TEST_TIMING)); alertTags.put(PolicyTag.DEV_FULL.getTag(), ""); alertTags.put(PolicyTag.QA_STD.getTag(), ""); alertTags.put(PolicyTag.QA_FULL.getTag(), ""); diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRule.java index cc93ea4f28f..29bb581eb37 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRule.java @@ -154,7 +154,8 @@ public class SqlInjectionOracleScanRule extends AbstractAppParamPlugin CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_05_SQLI)); + CommonAlertTag.WSTG_V42_INPV_05_SQLI, + CommonAlertTag.TEST_TIMING)); alertTags.put(PolicyTag.DEV_FULL.getTag(), ""); alertTags.put(PolicyTag.QA_STD.getTag(), ""); alertTags.put(PolicyTag.QA_FULL.getTag(), ""); diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRule.java index 415d5e2ab77..422886c9a05 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRule.java @@ -196,7 +196,8 @@ public class SqlInjectionPostgreScanRule extends AbstractAppParamPlugin CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_05_SQLI)); + CommonAlertTag.WSTG_V42_INPV_05_SQLI, + CommonAlertTag.TEST_TIMING)); alertTags.put(PolicyTag.DEV_FULL.getTag(), ""); alertTags.put(PolicyTag.QA_STD.getTag(), ""); alertTags.put(PolicyTag.QA_FULL.getTag(), ""); diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSqLiteScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSqLiteScanRule.java index 1f0fa2f9648..49374423f0b 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSqLiteScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSqLiteScanRule.java @@ -221,7 +221,8 @@ public class SqlInjectionSqLiteScanRule extends AbstractAppParamPlugin CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_05_SQLI)); + CommonAlertTag.WSTG_V42_INPV_05_SQLI, + CommonAlertTag.TEST_TIMING)); alertTags.put(PolicyTag.QA_FULL.getTag(), ""); ALERT_TAGS = Collections.unmodifiableMap(alertTags); } @@ -458,6 +459,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP .setOtherInfo(extraInfo) .setEvidence(matcher.group()) .setMessage(msgDelay) + .setTags(getNeededAlertTags(true)) .raise(); LOGGER.debug( @@ -600,6 +602,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP .setOtherInfo(extraInfo) .setEvidence(extraInfo) .setMessage(detectableDelayMessage) + .setTags(getNeededAlertTags(false)) .raise(); if (detectableDelayMessage != null) @@ -752,6 +755,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP .setOtherInfo(extraInfo) .setEvidence(versionNumber) .setMessage(unionAttackMessage) + .setTags(getNeededAlertTags(true)) .raise(); break unionLoops; } @@ -804,4 +808,13 @@ public int getWascId() { public Map getAlertTags() { return ALERT_TAGS; } + + private Map getNeededAlertTags(boolean isFeedbackbased) { + Map alertTags = new HashMap<>(); + alertTags.putAll(getAlertTags()); + if (isFeedbackbased) { + alertTags.remove(CommonAlertTag.TEST_TIMING.getTag()); + } + return alertTags; + } } diff --git a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRule.java b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRule.java index 65593f21ae7..e636cbca77e 100644 --- a/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRule.java +++ b/addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRule.java @@ -61,7 +61,8 @@ public class SstiBlindScanRule extends AbstractAppParamPlugin implements CommonA CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A03_INJECTION, CommonAlertTag.OWASP_2017_A01_INJECTION, - CommonAlertTag.WSTG_V42_INPV_18_SSTI)); + CommonAlertTag.WSTG_V42_INPV_18_SSTI, + CommonAlertTag.TEST_TIMING)); alertTags.put(ExtensionOast.OAST_ALERT_TAG_KEY, ExtensionOast.OAST_ALERT_TAG_VALUE); alertTags.put(PolicyTag.API.getTag(), ""); alertTags.put(PolicyTag.DEV_FULL.getTag(), ""); diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java index 611f17e31b5..c2d70cc6ee6 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java @@ -23,6 +23,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; @@ -95,7 +96,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(78))); assertThat(wasc, is(equalTo(31))); - assertThat(tags.size(), is(equalTo(10))); + assertThat(tags.size(), is(equalTo(11))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -378,6 +379,8 @@ void shouldHaveExpectedExampleAlert() { "The scan rule was able to retrieve the content of a file or " + "command by sending [a;cat /etc/passwd ] to the operating " + "system running this application."))); + Map tags = alert.getTags(); + assertThat(tags, not(hasKey(CommonAlertTag.TEST_TIMING.getTag()))); } private static class PayloadCollectorHandler extends NanoServerHandler { diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRuleUnitTest.java index 80d1cfd829f..ce0de90c1bb 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRuleUnitTest.java @@ -155,7 +155,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(89))); assertThat(wasc, is(equalTo(19))); - assertThat(tags.size(), is(equalTo(7))); + assertThat(tags.size(), is(equalTo(8))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -168,6 +168,7 @@ void shouldReturnExpectedMappings() { assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.SEQUENCE.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRuleUnitTest.java index 3ca0ac36de6..3085ee3143a 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRuleUnitTest.java @@ -150,7 +150,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(89))); assertThat(wasc, is(equalTo(19))); - assertThat(tags.size(), is(equalTo(7))); + assertThat(tags.size(), is(equalTo(8))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -163,6 +163,7 @@ void shouldReturnExpectedMappings() { assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.SEQUENCE.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRuleUnitTest.java index 1d8dd0fe8da..a80ff903843 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRuleUnitTest.java @@ -149,7 +149,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(89))); assertThat(wasc, is(equalTo(19))); - assertThat(tags.size(), is(equalTo(7))); + assertThat(tags.size(), is(equalTo(8))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -162,6 +162,7 @@ void shouldReturnExpectedMappings() { assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.SEQUENCE.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRuleUnitTest.java index d5a9103e241..92f8752a9f1 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRuleUnitTest.java @@ -145,7 +145,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(89))); assertThat(wasc, is(equalTo(19))); - assertThat(tags.size(), is(equalTo(7))); + assertThat(tags.size(), is(equalTo(8))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -158,6 +158,7 @@ void shouldReturnExpectedMappings() { assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.SEQUENCE.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRuleUnitTest.java index f893bc46bc0..2f55672218c 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRuleUnitTest.java @@ -158,7 +158,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(89))); assertThat(wasc, is(equalTo(19))); - assertThat(tags.size(), is(equalTo(7))); + assertThat(tags.size(), is(equalTo(8))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -171,6 +171,7 @@ void shouldReturnExpectedMappings() { assertThat(tags.containsKey(PolicyTag.QA_STD.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.SEQUENCE.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSQLiteScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSQLiteScanRuleUnitTest.java index a25ec55d3f5..44d2221852d 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSQLiteScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSQLiteScanRuleUnitTest.java @@ -22,7 +22,9 @@ import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import fi.iki.elonen.NanoHTTPD.IHTTPSession; @@ -114,6 +116,7 @@ protected Response serve(IHTTPSession session) { equalTo("case randomblob(100000) when not null then 1 else 1 end ")); assertThat(alertsRaised.get(0).getRisk(), equalTo(Alert.RISK_HIGH)); assertThat(alertsRaised.get(0).getConfidence(), equalTo(Alert.CONFIDENCE_MEDIUM)); + assertThat(alertsRaised.get(0).getTags(), not(hasKey(CommonAlertTag.TEST_TIMING.getTag()))); } @Test @@ -155,6 +158,7 @@ protected Response serve(IHTTPSession session) { assertThat(alertsRaised.get(0).getAttack(), startsWith("case randomblob(100")); assertThat(alertsRaised.get(0).getRisk(), equalTo(Alert.RISK_HIGH)); assertThat(alertsRaised.get(0).getConfidence(), equalTo(Alert.CONFIDENCE_MEDIUM)); + assertThat(alertsRaised.get(0).getTags(), is(hasKey(CommonAlertTag.TEST_TIMING.getTag()))); } @Test @@ -197,7 +201,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(89))); assertThat(wasc, is(equalTo(19))); - assertThat(tags.size(), is(equalTo(4))); + assertThat(tags.size(), is(equalTo(5))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -207,6 +211,7 @@ void shouldReturnExpectedMappings() { assertThat( tags.containsKey(CommonAlertTag.WSTG_V42_INPV_05_SQLI.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); diff --git a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRuleUnitTest.java b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRuleUnitTest.java index bb684afc5f5..78d709eaae1 100644 --- a/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRuleUnitTest.java +++ b/addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRuleUnitTest.java @@ -143,7 +143,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(1336))); assertThat(wasc, is(equalTo(20))); - assertThat(tags.size(), is(equalTo(8))); + assertThat(tags.size(), is(equalTo(9))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(true))); @@ -157,6 +157,7 @@ void shouldReturnExpectedMappings() { assertThat(tags.containsKey(PolicyTag.DEV_FULL.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.QA_FULL.getTag()), is(equalTo(true))); assertThat(tags.containsKey(PolicyTag.SEQUENCE.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); diff --git a/addOns/ascanrulesBeta/CHANGELOG.md b/addOns/ascanrulesBeta/CHANGELOG.md index 818c83b3ba7..4f27634fdc3 100644 --- a/addOns/ascanrulesBeta/CHANGELOG.md +++ b/addOns/ascanrulesBeta/CHANGELOG.md @@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - Replace usage of CWE-200 for the Insecure HTTP Method scan rule (Issue 8714). - Include exception message of failed attacks in the Server Side Request Forgery scan rule. +- The Shell Shock scan rule now has the TEST_TIMING alert tag. ### Fixed - Address potential/theoretical reDoS issue in the Insecure HTTP Method scan rule. diff --git a/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRule.java b/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRule.java index 68afde05af6..8cfd1f1bede 100644 --- a/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRule.java +++ b/addOns/ascanrulesBeta/src/main/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRule.java @@ -19,6 +19,7 @@ */ package org.zaproxy.zap.extension.ascanrulesBeta; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -46,7 +47,8 @@ public class ShellShockScanRule extends AbstractAppParamPlugin implements Common CommonAlertTag.toMap( CommonAlertTag.OWASP_2021_A06_VULN_COMP, CommonAlertTag.OWASP_2017_A09_VULN_COMP, - CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ); + CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ, + CommonAlertTag.TEST_TIMING); /** the logger object */ private static final Logger LOGGER = LogManager.getLogger(ShellShockScanRule.class); @@ -199,6 +201,7 @@ private AlertBuilder buildAlert(String paramName, String attack) { .setParam(paramName) .setAttack(attack) .setEvidence(EVIDENCE) + .setTags(getNeededAlertTags(true)) .setAlertRef(getId() + "-1"); } @@ -233,6 +236,15 @@ public Map getAlertTags() { return ALERT_TAGS; } + private Map getNeededAlertTags(boolean isReflectionBased) { + Map alertTags = new HashMap<>(); + alertTags.putAll(getAlertTags()); + if (isReflectionBased) { + alertTags.remove(CommonAlertTag.TEST_TIMING.getTag()); + } + return alertTags; + } + @Override public List getExampleAlerts() { return List.of( diff --git a/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRuleUnitTest.java b/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRuleUnitTest.java index 7db0211fc1c..369ce6ce884 100644 --- a/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRuleUnitTest.java +++ b/addOns/ascanrulesBeta/src/test/java/org/zaproxy/zap/extension/ascanrulesBeta/ShellShockScanRuleUnitTest.java @@ -22,7 +22,9 @@ import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import fi.iki.elonen.NanoHTTPD.IHTTPSession; import fi.iki.elonen.NanoHTTPD.Response; @@ -53,7 +55,7 @@ void shouldReturnExpectedMappings() { // Then assertThat(cwe, is(equalTo(78))); assertThat(wasc, is(equalTo(31))); - assertThat(tags.size(), is(equalTo(3))); + assertThat(tags.size(), is(equalTo(4))); assertThat( tags.containsKey(CommonAlertTag.OWASP_2021_A06_VULN_COMP.getTag()), is(equalTo(true))); @@ -63,6 +65,7 @@ void shouldReturnExpectedMappings() { assertThat( tags.containsKey(CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); assertThat( tags.get(CommonAlertTag.OWASP_2021_A06_VULN_COMP.getTag()), is(equalTo(CommonAlertTag.OWASP_2021_A06_VULN_COMP.getValue()))); @@ -82,8 +85,10 @@ void shouldHaveExpectedExampleAlerts() { assertThat(alerts.size(), is(equalTo(2))); Alert alert = alerts.get(0); assertThat(alert.getAlertRef(), is(equalTo("10048-1"))); + assertThat(alert.getTags(), not(hasKey(CommonAlertTag.TEST_TIMING.getTag()))); Alert timingAlert = alerts.get(1); assertThat(timingAlert.getAlertRef(), is(equalTo("10048-2"))); + assertThat(timingAlert.getTags(), is(hasKey(CommonAlertTag.TEST_TIMING.getTag()))); } @Test diff --git a/addOns/sqliplugin/CHANGELOG.md b/addOns/sqliplugin/CHANGELOG.md index 4dd0cf6a924..4a9a177f9ec 100644 --- a/addOns/sqliplugin/CHANGELOG.md +++ b/addOns/sqliplugin/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - Update minimum ZAP version to 2.16.0. - Maintenance changes. +- The scan rule now has the "TEST_TIMING" alert tag. ## [15] - 2021-10-20 ### Fixed diff --git a/addOns/sqliplugin/sqliplugin.gradle.kts b/addOns/sqliplugin/sqliplugin.gradle.kts index e5c4cf537b1..d1b175156ac 100644 --- a/addOns/sqliplugin/sqliplugin.gradle.kts +++ b/addOns/sqliplugin/sqliplugin.gradle.kts @@ -35,6 +35,8 @@ dependencies { zapAddOn("commonlib") implementation("org.jdom:jdom:2.0.2") + + testImplementation(project(":testutils")) } spotless { diff --git a/addOns/sqliplugin/src/main/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRule.java b/addOns/sqliplugin/src/main/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRule.java index 8f5c35a15f6..0977375c6d1 100644 --- a/addOns/sqliplugin/src/main/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRule.java +++ b/addOns/sqliplugin/src/main/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRule.java @@ -22,6 +22,8 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -64,10 +66,19 @@ public class SQLInjectionScanRule extends AbstractAppParamPlugin { // Payload used for checking of existence of IDS/WAF (dumber the better) // IDS_WAF_CHECK_PAYLOAD = "AND 1=1 UNION ALL SELECT 1,2,3,table_name FROM // information_schema.tables" - private static final Map ALERT_TAGS = - CommonAlertTag.toMap( - CommonAlertTag.OWASP_2021_A03_INJECTION, - CommonAlertTag.OWASP_2017_A01_INJECTION); + private static final Map ALERT_TAGS; + + static { + Map alertTags = + new HashMap<>( + CommonAlertTag.toMap( + CommonAlertTag.OWASP_2021_A03_INJECTION, + CommonAlertTag.OWASP_2017_A01_INJECTION, + CommonAlertTag.WSTG_V42_INPV_05_SQLI, + CommonAlertTag.TEST_TIMING)); + ALERT_TAGS = Collections.unmodifiableMap(alertTags); + } + // ------------------------------------------------------------------ // Configuration properties // ------------------------------------------------------------------ @@ -778,7 +789,7 @@ public void scan(HttpMessage msg, String parameter, String value) { parameter); // Alert the vulnerability to the main core - raiseAlert(title, parameter, reqPayload, info, tempMsg); + raiseAlert(title, parameter, reqPayload, info, tempMsg, true); // Close the boundary/where iteration injectable = true; @@ -865,7 +876,7 @@ public void scan(HttpMessage msg, String parameter, String value) { reqPayload, parameter); - raiseAlert(title, parameter, reqPayload, info, tempMsg); + raiseAlert(title, parameter, reqPayload, info, tempMsg, true); // Close the boundary/where iteration injectable = true; @@ -959,7 +970,7 @@ public void scan(HttpMessage msg, String parameter, String value) { reqPayload, parameter); - raiseAlert(title, parameter, reqPayload, info, tempMsg); + raiseAlert(title, parameter, reqPayload, info, tempMsg, false); // Close the boundary/where iteration injectable = true; @@ -1031,7 +1042,8 @@ public void scan(HttpMessage msg, String parameter, String value) { parameter, engine.getExploitPayload(), info, - engine.getExploitMessage()); + engine.getExploitMessage(), + true); // Close the boundary/where iteration injectable = true; @@ -1080,7 +1092,8 @@ private void raiseAlert( String parameter, String payload, String otherInfo, - HttpMessage message) { + HttpMessage message, + boolean feedbackBased) { newAlert() .setConfidence(Alert.CONFIDENCE_MEDIUM) .setName(Constant.messages.getString(ALERT_MESSAGE_PREFIX + "name", subTitle)) @@ -1088,6 +1101,7 @@ private void raiseAlert( .setAttack(payload) .setOtherInfo(otherInfo) .setMessage(message) + .setTags(getNeededAlertTags(feedbackBased)) .raise(); } @@ -1096,6 +1110,15 @@ public Map getAlertTags() { return ALERT_TAGS; } + private Map getNeededAlertTags(boolean feedbackBased) { + Map alertTags = new HashMap<>(); + alertTags.putAll(getAlertTags()); + if (feedbackBased) { + alertTags.remove(CommonAlertTag.TEST_TIMING.getTag()); + } + return alertTags; + } + /** * Launch the requested payload. If null values sent in paramName or payload launch again the * original message diff --git a/addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/ActiveScannerTest.java b/addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/ActiveScannerTest.java new file mode 100644 index 00000000000..3f971426e3f --- /dev/null +++ b/addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/ActiveScannerTest.java @@ -0,0 +1,31 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2024 The ZAP Development Team + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.zaproxy.zap.extension.sqliplugin; + +import org.parosproxy.paros.core.scanner.AbstractPlugin; +import org.zaproxy.zap.testutils.ActiveScannerTestUtils; + +abstract class ActiveScannerTest extends ActiveScannerTestUtils { + + @Override + protected void setUpMessages() { + mockMessages(new ExtensionSQLiPlugin()); + } +} diff --git a/addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRuleUnitTest.java b/addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRuleUnitTest.java new file mode 100644 index 00000000000..c7ce27f0199 --- /dev/null +++ b/addOns/sqliplugin/src/test/java/org/zaproxy/zap/extension/sqliplugin/SQLInjectionScanRuleUnitTest.java @@ -0,0 +1,80 @@ +/* + * Zed Attack Proxy (ZAP) and its related class files. + * + * ZAP is an HTTP/HTTPS proxy for assessing web application security. + * + * Copyright 2024 The ZAP Development Team + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.zaproxy.zap.extension.sqliplugin; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; + +import java.util.Map; +import org.junit.jupiter.api.Test; +import org.parosproxy.paros.core.scanner.Plugin.AttackStrength; +import org.zaproxy.addon.commonlib.CommonAlertTag; + +class SQLInjectionScanRuleUnitTest extends ActiveScannerTest { + + @Override + protected SQLInjectionScanRule createScanner() { + return new SQLInjectionScanRule(); + } + + @Override + protected int getRecommendMaxNumberMessagesPerParam(AttackStrength strength) { + switch (strength) { + case LOW, MEDIUM: + default: + return 222; + case HIGH: + return 915; + case INSANE: + return 3627; + } + } + + @Test + void shouldReturnExpectedMappings() { + // Given / When + int cwe = rule.getCweId(); + int wasc = rule.getWascId(); + Map tags = rule.getAlertTags(); + // Then + assertThat(cwe, is(equalTo(89))); + assertThat(wasc, is(equalTo(19))); + assertThat(tags.size(), is(equalTo(4))); + assertThat( + tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), + is(equalTo(true))); + assertThat( + tags.containsKey(CommonAlertTag.OWASP_2017_A01_INJECTION.getTag()), + is(equalTo(true))); + assertThat( + tags.containsKey(CommonAlertTag.WSTG_V42_INPV_05_SQLI.getTag()), is(equalTo(true))); + assertThat(tags.containsKey(CommonAlertTag.TEST_TIMING.getTag()), is(equalTo(true))); + assertThat( + tags.get(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()), + is(equalTo(CommonAlertTag.OWASP_2021_A03_INJECTION.getValue()))); + assertThat( + tags.get(CommonAlertTag.OWASP_2017_A01_INJECTION.getTag()), + is(equalTo(CommonAlertTag.OWASP_2017_A01_INJECTION.getValue()))); + assertThat( + tags.get(CommonAlertTag.WSTG_V42_INPV_05_SQLI.getTag()), + is(equalTo(CommonAlertTag.WSTG_V42_INPV_05_SQLI.getValue()))); + } +}