Skip to content

Commit 2b2f389

Browse files
kingthorinkingthorin
committed
various: Add TEST_TIMING alert tag
Update: CHANGELOGs, scan rules, unittests. Signed-off-by: kingthorin <[email protected]> # Conflicts: # addOns/ascanrules/CHANGELOG.md
1 parent dc9bcb0 commit 2b2f389

25 files changed

+191
-36
lines changed

‎addOns/ascanrules/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1212
- SQL Injection scan rule to start using ComparableResponse - part of the work to reduce False Positives.
1313
- Depends on an updated version of the Common Library add-on.
1414
- Due to it being 2025 and the mass adoption of HTTPS: De-prioritized plain HTTP payloads in the External Redirect scan rule.
15+
- Scan rules which execute time based attacks now include the "TEST_TIMING" alert tag.
1516

1617
### Fixed
1718
- SQL Injection scan rule to treat a 500 response to an SQLi attack as a likely vulnerability.

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

Lines changed: 20 additions & 13 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(), "");
@@ -368,6 +369,16 @@ public Map<String, String> getAlertTags() {
368369
return ALERT_TAGS;
369370
}
370371

372+
private Map<String, String> getNeededAlertTags(TestType type) {
373+
if (TestType.FEEDBACK.equals(type)) {
374+
Map<String, String> alertTags = new HashMap<>();
375+
alertTags.putAll(getAlertTags());
376+
alertTags.remove(CommonAlertTag.TEST_TIMING.getTag());
377+
return alertTags;
378+
}
379+
return ALERT_TAGS;
380+
}
381+
371382
@Override
372383
public int getCweId() {
373384
return 78;
@@ -583,9 +594,9 @@ private boolean testCommandInjection(
583594
"[OS Command Injection Found] on parameter [{}] with value [{}]",
584595
paramName,
585596
paramValue);
586-
String otherInfo = getOtherInfo(TestType.FEEDBACK, paramValue);
587597

588-
buildAlert(paramName, paramValue, matcher.group(), otherInfo, msg).raise();
598+
buildAlert(paramName, paramValue, matcher.group(), TestType.FEEDBACK, msg)
599+
.raise();
589600

590601
// All done. No need to look for vulnerabilities on subsequent
591602
// payloads on the same request (to reduce performance impact)
@@ -668,10 +679,9 @@ private boolean testCommandInjection(
668679
"[Blind OS Command Injection Found] on parameter [{}] with value [{}]",
669680
paramName,
670681
paramValue);
671-
String otherInfo = getOtherInfo(TestType.TIME, paramValue);
672682

673683
// just attach this alert to the last sent message
674-
buildAlert(paramName, paramValue, "", otherInfo, message.get()).raise();
684+
buildAlert(paramName, paramValue, "", TestType.TIME, message.get()).raise();
675685

676686
// All done. No need to look for vulnerabilities on subsequent
677687
// payloads on the same request (to reduce performance impact)
@@ -720,25 +730,22 @@ private static String insertUninitVar(String cmd) {
720730
}
721731

722732
private AlertBuilder buildAlert(
723-
String param, String attack, String evidence, String otherInfo, HttpMessage msg) {
733+
String param, String attack, String evidence, TestType type, HttpMessage msg) {
734+
String otherInfo = getOtherInfo(type, attack);
724735
return newAlert()
725736
.setConfidence(Alert.CONFIDENCE_MEDIUM)
726737
.setParam(param)
727738
.setAttack(attack)
728739
.setEvidence(evidence)
729740
.setMessage(msg)
730-
.setOtherInfo(otherInfo);
741+
.setOtherInfo(otherInfo)
742+
.setTags(getNeededAlertTags(type));
731743
}
732744

733745
@Override
734746
public List<Alert> getExampleAlerts() {
735747
return List.of(
736-
buildAlert(
737-
"qry",
738-
"a;cat /etc/passwd ",
739-
"root:x:0:0",
740-
getOtherInfo(TestType.FEEDBACK, "a;cat /etc/passwd "),
741-
null)
748+
buildAlert("qry", "a;cat /etc/passwd ", "root:x:0:0", TestType.FEEDBACK, null)
742749
.build());
743750
}
744751
}

‎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: 15 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
alertTags.put(PolicyTag.PENTEST.getTag(), "");
227228
ALERT_TAGS = Collections.unmodifiableMap(alertTags);
@@ -459,6 +460,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP
459460
.setOtherInfo(extraInfo)
460461
.setEvidence(matcher.group())
461462
.setMessage(msgDelay)
463+
.setTags(getNeededAlertTags(true))
462464
.raise();
463465

464466
LOGGER.debug(
@@ -601,6 +603,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP
601603
.setOtherInfo(extraInfo)
602604
.setEvidence(extraInfo)
603605
.setMessage(detectableDelayMessage)
606+
.setTags(getNeededAlertTags(false))
604607
.raise();
605608

606609
if (detectableDelayMessage != null)
@@ -753,6 +756,7 @@ public void scan(HttpMessage originalMessage, String paramName, String originalP
753756
.setOtherInfo(extraInfo)
754757
.setEvidence(versionNumber)
755758
.setMessage(unionAttackMessage)
759+
.setTags(getNeededAlertTags(true))
756760
.raise();
757761
break unionLoops;
758762
}
@@ -805,4 +809,14 @@ public int getWascId() {
805809
public Map<String, String> getAlertTags() {
806810
return ALERT_TAGS;
807811
}
812+
813+
private Map<String, String> getNeededAlertTags(boolean isFeedbackBased) {
814+
if (isFeedbackBased) {
815+
Map<String, String> alertTags = new HashMap<>();
816+
alertTags.putAll(getAlertTags());
817+
alertTags.remove(CommonAlertTag.TEST_TIMING.getTag());
818+
return alertTags;
819+
}
820+
return ALERT_TAGS;
821+
}
808822
}

‎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(11)));
99+
assertThat(tags.size(), is(equalTo(12)));
99100
assertThat(
100101
tags.containsKey(CommonAlertTag.OWASP_2021_A03_INJECTION.getTag()),
101102
is(equalTo(true)));
@@ -379,6 +380,8 @@ void shouldHaveExpectedExampleAlert() {
379380
"The scan rule was able to retrieve the content of a file or "
380381
+ "command by sending [a;cat /etc/passwd ] to the operating "
381382
+ "system running this application.")));
383+
Map<String, String> tags = alert.getTags();
384+
assertThat(tags, not(hasKey(CommonAlertTag.TEST_TIMING.getTag())));
382385
}
383386

384387
private static class PayloadCollectorHandler extends NanoServerHandler {

0 commit comments

Comments
 (0)