Skip to content

Commit 58f25e1

Browse files
committed
various: Add TEST_TIMING alert tag
Update: CHANGELOGs, scan rules, unittests. Signed-off-by: kingthorin <[email protected]>
1 parent ca584fe commit 58f25e1

25 files changed

+245
-31
lines changed

addOns/ascanrules/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1313
- Add the `OUT_OF_BAND` alert tag to the following scan rules:
1414
- Server Side Template Injection (Blind)
1515
- XML External Entity Attack
16+
- Scan rules which execute time based attacks now include the "TEST_TIMING" alert tag.
1617

1718
### Fixed
1819
- A situation where the Server-Side Template Injection (SSTI) scan rule might result in false positives related to the Go payloads (Issue 8622).

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRule.java

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ public class CommandInjectionScanRule extends AbstractAppParamPlugin
9898
CommonAlertTag.toMap(
9999
CommonAlertTag.OWASP_2021_A03_INJECTION,
100100
CommonAlertTag.OWASP_2017_A01_INJECTION,
101-
CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ));
101+
CommonAlertTag.WSTG_V42_INPV_12_COMMAND_INJ,
102+
CommonAlertTag.TEST_TIMING));
102103
alertTags.put(PolicyTag.API.getTag(), "");
103104
alertTags.put(PolicyTag.DEV_CICD.getTag(), "");
104105
alertTags.put(PolicyTag.DEV_STD.getTag(), "");
@@ -367,6 +368,15 @@ public Map<String, String> getAlertTags() {
367368
return ALERT_TAGS;
368369
}
369370

371+
private Map<String, String> getNeededAlertTags(TestType type) {
372+
Map<String, String> alertTags = new HashMap<>();
373+
alertTags.putAll(getAlertTags());
374+
if (TestType.FEEDBACK.equals(type)) {
375+
alertTags.remove(CommonAlertTag.TEST_TIMING.getTag());
376+
}
377+
return alertTags;
378+
}
379+
370380
@Override
371381
public int getCweId() {
372382
return 78;
@@ -584,7 +594,14 @@ private boolean testCommandInjection(
584594
paramValue);
585595
String otherInfo = getOtherInfo(TestType.FEEDBACK, paramValue);
586596

587-
buildAlert(paramName, paramValue, matcher.group(), otherInfo, msg).raise();
597+
buildAlert(
598+
paramName,
599+
paramValue,
600+
matcher.group(),
601+
otherInfo,
602+
TestType.FEEDBACK,
603+
msg)
604+
.raise();
588605

589606
// All done. No need to look for vulnerabilities on subsequent
590607
// payloads on the same request (to reduce performance impact)
@@ -670,7 +687,8 @@ private boolean testCommandInjection(
670687
String otherInfo = getOtherInfo(TestType.TIME, paramValue);
671688

672689
// just attach this alert to the last sent message
673-
buildAlert(paramName, paramValue, "", otherInfo, message.get()).raise();
690+
buildAlert(paramName, paramValue, "", otherInfo, TestType.TIME, message.get())
691+
.raise();
674692

675693
// All done. No need to look for vulnerabilities on subsequent
676694
// payloads on the same request (to reduce performance impact)
@@ -719,14 +737,20 @@ private static String insertUninitVar(String cmd) {
719737
}
720738

721739
private AlertBuilder buildAlert(
722-
String param, String attack, String evidence, String otherInfo, HttpMessage msg) {
740+
String param,
741+
String attack,
742+
String evidence,
743+
String otherInfo,
744+
TestType type,
745+
HttpMessage msg) {
723746
return newAlert()
724747
.setConfidence(Alert.CONFIDENCE_MEDIUM)
725748
.setParam(param)
726749
.setAttack(attack)
727750
.setEvidence(evidence)
728751
.setMessage(msg)
729-
.setOtherInfo(otherInfo);
752+
.setOtherInfo(otherInfo)
753+
.setTags(getNeededAlertTags(type));
730754
}
731755

732756
@Override
@@ -737,6 +761,7 @@ public List<Alert> getExampleAlerts() {
737761
"a;cat /etc/passwd ",
738762
"root:x:0:0",
739763
getOtherInfo(TestType.FEEDBACK, "a;cat /etc/passwd "),
764+
TestType.FEEDBACK,
740765
null)
741766
.build());
742767
}

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionHypersonicScanRule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ public class SqlInjectionHypersonicScanRule extends AbstractAppParamPlugin
199199
CommonAlertTag.toMap(
200200
CommonAlertTag.OWASP_2021_A03_INJECTION,
201201
CommonAlertTag.OWASP_2017_A01_INJECTION,
202-
CommonAlertTag.WSTG_V42_INPV_05_SQLI));
202+
CommonAlertTag.WSTG_V42_INPV_05_SQLI,
203+
CommonAlertTag.TEST_TIMING));
203204
alertTags.put(PolicyTag.DEV_FULL.getTag(), "");
204205
alertTags.put(PolicyTag.QA_STD.getTag(), "");
205206
alertTags.put(PolicyTag.QA_FULL.getTag(), "");

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMsSqlScanRule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,8 @@ public class SqlInjectionMsSqlScanRule extends AbstractAppParamPlugin
144144
CommonAlertTag.toMap(
145145
CommonAlertTag.OWASP_2021_A03_INJECTION,
146146
CommonAlertTag.OWASP_2017_A01_INJECTION,
147-
CommonAlertTag.WSTG_V42_INPV_05_SQLI));
147+
CommonAlertTag.WSTG_V42_INPV_05_SQLI,
148+
CommonAlertTag.TEST_TIMING));
148149
alertTags.put(PolicyTag.DEV_FULL.getTag(), "");
149150
alertTags.put(PolicyTag.QA_STD.getTag(), "");
150151
alertTags.put(PolicyTag.QA_FULL.getTag(), "");

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionMySqlScanRule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,8 @@ public class SqlInjectionMySqlScanRule extends AbstractAppParamPlugin
218218
CommonAlertTag.toMap(
219219
CommonAlertTag.OWASP_2021_A03_INJECTION,
220220
CommonAlertTag.OWASP_2017_A01_INJECTION,
221-
CommonAlertTag.WSTG_V42_INPV_05_SQLI));
221+
CommonAlertTag.WSTG_V42_INPV_05_SQLI,
222+
CommonAlertTag.TEST_TIMING));
222223
alertTags.put(PolicyTag.DEV_FULL.getTag(), "");
223224
alertTags.put(PolicyTag.QA_STD.getTag(), "");
224225
alertTags.put(PolicyTag.QA_FULL.getTag(), "");

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionOracleScanRule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ public class SqlInjectionOracleScanRule extends AbstractAppParamPlugin
154154
CommonAlertTag.toMap(
155155
CommonAlertTag.OWASP_2021_A03_INJECTION,
156156
CommonAlertTag.OWASP_2017_A01_INJECTION,
157-
CommonAlertTag.WSTG_V42_INPV_05_SQLI));
157+
CommonAlertTag.WSTG_V42_INPV_05_SQLI,
158+
CommonAlertTag.TEST_TIMING));
158159
alertTags.put(PolicyTag.DEV_FULL.getTag(), "");
159160
alertTags.put(PolicyTag.QA_STD.getTag(), "");
160161
alertTags.put(PolicyTag.QA_FULL.getTag(), "");

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionPostgreScanRule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,8 @@ public class SqlInjectionPostgreScanRule extends AbstractAppParamPlugin
196196
CommonAlertTag.toMap(
197197
CommonAlertTag.OWASP_2021_A03_INJECTION,
198198
CommonAlertTag.OWASP_2017_A01_INJECTION,
199-
CommonAlertTag.WSTG_V42_INPV_05_SQLI));
199+
CommonAlertTag.WSTG_V42_INPV_05_SQLI,
200+
CommonAlertTag.TEST_TIMING));
200201
alertTags.put(PolicyTag.DEV_FULL.getTag(), "");
201202
alertTags.put(PolicyTag.QA_STD.getTag(), "");
202203
alertTags.put(PolicyTag.QA_FULL.getTag(), "");

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SqlInjectionSqLiteScanRule.java

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,8 @@ public class SqlInjectionSqLiteScanRule extends AbstractAppParamPlugin
221221
CommonAlertTag.toMap(
222222
CommonAlertTag.OWASP_2021_A03_INJECTION,
223223
CommonAlertTag.OWASP_2017_A01_INJECTION,
224-
CommonAlertTag.WSTG_V42_INPV_05_SQLI));
224+
CommonAlertTag.WSTG_V42_INPV_05_SQLI,
225+
CommonAlertTag.TEST_TIMING));
225226
alertTags.put(PolicyTag.QA_FULL.getTag(), "");
226227
ALERT_TAGS = Collections.unmodifiableMap(alertTags);
227228
}
@@ -458,6 +459,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP
458459
.setOtherInfo(extraInfo)
459460
.setEvidence(matcher.group())
460461
.setMessage(msgDelay)
462+
.setTags(getNeededAlertTags(true))
461463
.raise();
462464

463465
LOGGER.debug(
@@ -600,6 +602,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP
600602
.setOtherInfo(extraInfo)
601603
.setEvidence(extraInfo)
602604
.setMessage(detectableDelayMessage)
605+
.setTags(getNeededAlertTags(false))
603606
.raise();
604607

605608
if (detectableDelayMessage != null)
@@ -752,6 +755,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP
752755
.setOtherInfo(extraInfo)
753756
.setEvidence(versionNumber)
754757
.setMessage(unionAttackMessage)
758+
.setTags(getNeededAlertTags(true))
755759
.raise();
756760
break unionLoops;
757761
}
@@ -804,4 +808,13 @@ public int getWascId() {
804808
public Map<String, String> getAlertTags() {
805809
return ALERT_TAGS;
806810
}
811+
812+
private Map<String, String> getNeededAlertTags(boolean isFeedbackbased) {
813+
Map<String, String> alertTags = new HashMap<>();
814+
alertTags.putAll(getAlertTags());
815+
if (isFeedbackbased) {
816+
alertTags.remove(CommonAlertTag.TEST_TIMING.getTag());
817+
}
818+
return alertTags;
819+
}
807820
}

addOns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/SstiBlindScanRule.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,8 @@ public class SstiBlindScanRule extends AbstractAppParamPlugin implements CommonA
6161
CommonAlertTag.toMap(
6262
CommonAlertTag.OWASP_2021_A03_INJECTION,
6363
CommonAlertTag.OWASP_2017_A01_INJECTION,
64-
CommonAlertTag.WSTG_V42_INPV_18_SSTI));
64+
CommonAlertTag.WSTG_V42_INPV_18_SSTI,
65+
CommonAlertTag.TEST_TIMING));
6566
alertTags.put(ExtensionOast.OAST_ALERT_TAG_KEY, ExtensionOast.OAST_ALERT_TAG_VALUE);
6667
alertTags.put(PolicyTag.API.getTag(), "");
6768
alertTags.put(PolicyTag.DEV_FULL.getTag(), "");

addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/CommandInjectionScanRuleUnitTest.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import static org.hamcrest.MatcherAssert.assertThat;
2424
import static org.hamcrest.Matchers.containsString;
2525
import static org.hamcrest.Matchers.equalTo;
26+
import static org.hamcrest.Matchers.hasKey;
2627
import static org.hamcrest.Matchers.hasSize;
2728
import static org.hamcrest.Matchers.is;
2829
import static org.hamcrest.Matchers.not;
@@ -95,7 +96,7 @@ void shouldReturnExpectedMappings() {
9596
// Then
9697
assertThat(cwe, is(equalTo(78)));
9798
assertThat(wasc, is(equalTo(31)));
98-
assertThat(tags.size(), is(equalTo(10)));
99+
assertThat(tags.size(), is(equalTo(11)));
99100
assertThat(
100101
tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()),
101102
is(equalTo(true)));
@@ -378,6 +379,8 @@ void shouldHaveExpectedExampleAlert() {
378379
"The scan rule was able to retrieve the content of a file or "
379380
+ "command by sending [a;cat /etc/passwd ] to the operating "
380381
+ "system running this application.")));
382+
Map<String, String> tags = alert.getTags();
383+
assertThat(tags, not(hasKey(CommonAlertTag.TEST_TIMING.getTag())));
381384
}
382385

383386
private static class PayloadCollectorHandler extends NanoServerHandler {

0 commit comments

Comments
 (0)