Skip to content

Commit cb59ded

Browse files
kingthorinkingthorin
committed
ascanrules: Path Traversal add details for dir match Alerts & reduce FPs
- CHANGELOG > Added change note. - Message.properties > Added key/value pair supporting the new Alert details. - PathTraversalScanRule > Updated to include Other Info on Alerts when applicable, and pre-check the original message response to reduce false positives. - PathTraversalScanRuleUnitTest > Updated to assert Other Info or lack thereof where applicable, also assure appropriate skipping due to pre-conditions. Signed-off-by: kingthorin <[email protected]> # Conflicts: # addOns/ascanrules/CHANGELOG.md
1 parent e5fe1b8 commit cb59ded

File tree

4 files changed

+189
-49
lines changed

4 files changed

+189
-49
lines changed

‎addOns/ascanrules/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1111
### Changed
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.
14+
- The Path Traversal scan rule now includes further details when directory matches are made and pre-checks the original message to reduce false positives (Issue 8379).
1415

1516
### Fixed
1617
- 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/PathTraversalScanRule.java‎

Lines changed: 74 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
8181
* Windows local file targets and detection pattern
8282
*/
8383
private static final ContentsMatcher WIN_PATTERN =
84-
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"));
84+
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"), Tech.Windows);
85+
private static final String WIN_DIR_EVIDENCE = "Windows";
8586
private static final String[] WIN_LOCAL_FILE_TARGETS = {
8687
// Absolute Windows file retrieval (we suppose C:\\)
8788
"c:/Windows/system.ini",
@@ -130,7 +131,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
130131
*/
131132
// Dot used to match 'x' or '!' (used in AIX)
132133
private static final ContentsMatcher NIX_PATTERN =
133-
new PatternContentsMatcher(Pattern.compile("root:.:0:0"));
134+
new PatternContentsMatcher(Pattern.compile("root:.:0:0"), Tech.Linux);
135+
private static final String NIX_DIR_EVIDENCE = "etc";
134136
private static final String[] NIX_LOCAL_FILE_TARGETS = {
135137
// Absolute file retrieval
136138
"/etc/passwd",
@@ -156,10 +158,9 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
156158
// ..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252Fetc%252Fpasswd%2500.jpg
157159
};
158160

159-
/*
160-
* Windows/Unix/Linux/etc. local directory targets and detection pattern
161-
*/
162-
private static final ContentsMatcher DIR_PATTERN = new DirNamesContentsMatcher();
161+
private static final ContentsMatcher NIX_DIR_MATCHER = new DirNamesContentsMatcher(Tech.Linux);
162+
private static final ContentsMatcher WIN_DIR_MATCHER =
163+
new DirNamesContentsMatcher(Tech.Windows);
163164
private static final String[] WIN_LOCAL_DIR_TARGETS = {
164165
"c:/",
165166
"c:\\",
@@ -181,17 +182,19 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
181182
"/",
182183
"../../../../../../../../../../../../../../../../",
183184
"/../../../../../../../../../../../../../../../../",
184-
"file:///",
185+
"file:///"
185186
};
186187

187188
private static final ContentsMatcher WAR_PATTERN =
188-
new PatternContentsMatcher(Pattern.compile("</web-app>"));
189+
new PatternContentsMatcher(Pattern.compile("</web-app>"), Tech.Tomcat);
189190

190191
/*
191192
* Standard local file prefixes
192193
*/
193194
private static final String[] LOCAL_FILE_RELATIVE_PREFIXES = {"", "/", "\\"};
194195

196+
private static final List<String> DIR_EVIDENCE_LIST =
197+
List.of(NIX_DIR_EVIDENCE, WIN_DIR_EVIDENCE);
195198
/*
196199
* details of the vulnerability which we are attempting to find
197200
*/
@@ -342,7 +345,6 @@ public void scan(HttpMessage msg, String param, String value) {
342345

343346
// Check 2: Start detection for *NIX patterns
344347
if (inScope(Tech.Linux) || inScope(Tech.MacOS)) {
345-
346348
for (int h = 0; h < nixCount; h++) {
347349

348350
// Check if a there was a finding or the scan has been stopped
@@ -384,10 +386,9 @@ public void scan(HttpMessage msg, String param, String value) {
384386
// Check 3: Detect if this page is a directory browsing component
385387
if (inScope(Tech.Linux) || inScope(Tech.MacOS)) {
386388
for (int h = 0; h < nixDirCount; h++) {
387-
388389
// Check if a there was a finding or the scan has been stopped
389390
// if yes dispose resources and exit
390-
if (sendAndCheckPayload(param, NIX_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
391+
if (sendAndCheckPayload(param, NIX_LOCAL_DIR_TARGETS[h], NIX_DIR_MATCHER, 3)
391392
|| isStop()) {
392393
// Dispose all resources
393394
// Exit the scan rule
@@ -397,7 +398,7 @@ public void scan(HttpMessage msg, String param, String value) {
397398
}
398399
if (inScope(Tech.Windows)) {
399400
for (int h = 0; h < winDirCount; h++) {
400-
if (sendAndCheckPayload(param, WIN_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
401+
if (sendAndCheckPayload(param, WIN_LOCAL_DIR_TARGETS[h], WIN_DIR_MATCHER, 3)
401402
|| isStop()) {
402403
// Dispose all resources
403404
// Exit the scan rule
@@ -659,12 +660,23 @@ private AlertBuilder createUnmatchedAlert(String param, String attack) {
659660

660661
private AlertBuilder createMatchedAlert(
661662
String param, String attack, String evidence, int check) {
662-
return newAlert()
663-
.setConfidence(Alert.CONFIDENCE_MEDIUM)
664-
.setParam(param)
665-
.setAttack(attack)
666-
.setEvidence(evidence)
667-
.setAlertRef(getId() + "-" + check);
663+
AlertBuilder builder =
664+
newAlert()
665+
.setConfidence(Alert.CONFIDENCE_MEDIUM)
666+
.setParam(param)
667+
.setAttack(attack)
668+
.setEvidence(evidence)
669+
.setAlertRef(getId() + "-" + check);
670+
if (DIR_EVIDENCE_LIST.contains(evidence)) {
671+
builder.setOtherInfo(
672+
Constant.messages.getString(
673+
MESSAGE_PREFIX + "info",
674+
evidence,
675+
evidence.equals(WIN_DIR_EVIDENCE)
676+
? DirNamesContentsMatcher.WIN_MATCHES
677+
: DirNamesContentsMatcher.NIX_MATCHES));
678+
}
679+
return builder;
668680
}
669681

670682
@Override
@@ -690,7 +702,7 @@ private static class PatternContentsMatcher implements ContentsMatcher {
690702

691703
private final Pattern pattern;
692704

693-
public PatternContentsMatcher(Pattern pattern) {
705+
public PatternContentsMatcher(Pattern pattern, Tech tech) {
694706
this.pattern = pattern;
695707
}
696708

@@ -706,46 +718,61 @@ public String match(String contents) {
706718

707719
private static class DirNamesContentsMatcher implements ContentsMatcher {
708720

721+
private static final String NIX_MATCHES =
722+
String.join(", ", List.of("proc", NIX_DIR_EVIDENCE, "boot", "tmp", "home"));
723+
private static final String WIN_MATCHES =
724+
String.join(", ", List.of(WIN_DIR_EVIDENCE, "Program Files"));
725+
private static final Pattern PROC_PATT =
726+
Pattern.compile(
727+
"(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
728+
private static final Pattern ETC_PATT =
729+
Pattern.compile(
730+
"(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
731+
private static final Pattern BOOT_PATT =
732+
Pattern.compile(
733+
"(?:^|\\W)boot(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
734+
private static final Pattern TMP_PATT =
735+
Pattern.compile(
736+
"(?:^|\\W)tmp(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
737+
private static final Pattern HOME_PATT =
738+
Pattern.compile(
739+
"(?:^|\\W)home(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
740+
741+
private Tech tech;
742+
743+
public DirNamesContentsMatcher(Tech tech) {
744+
this.tech = tech;
745+
}
746+
709747
@Override
710748
public String match(String contents) {
711-
String result = matchNixDirectories(contents);
712-
if (result != null) {
713-
return result;
749+
if (this.tech == Tech.Linux) {
750+
return matchNixDirectories(contents);
751+
}
752+
if (this.tech == Tech.Windows) {
753+
return matchWinDirectories(contents);
714754
}
715-
return matchWinDirectories(contents);
755+
return null;
716756
}
717757

718758
private static String matchNixDirectories(String contents) {
719-
Pattern procPattern =
720-
Pattern.compile("(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
721-
Pattern etcPattern = Pattern.compile("(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
722-
Pattern bootPattern =
723-
Pattern.compile("(?:^|\\W)boot(?:\\W|$)", Pattern.CASE_INSENSITIVE);
724-
Pattern tmpPattern = Pattern.compile("(?:^|\\W)tmp(?:\\W|$)", Pattern.CASE_INSENSITIVE);
725-
Pattern homePattern =
726-
Pattern.compile("(?:^|\\W)home(?:\\W|$)", Pattern.CASE_INSENSITIVE);
727-
728-
Matcher procMatcher = procPattern.matcher(contents);
729-
Matcher etcMatcher = etcPattern.matcher(contents);
730-
Matcher bootMatcher = bootPattern.matcher(contents);
731-
Matcher tmpMatcher = tmpPattern.matcher(contents);
732-
Matcher homeMatcher = homePattern.matcher(contents);
733-
734-
if (procMatcher.find()
735-
&& etcMatcher.find()
736-
&& bootMatcher.find()
737-
&& tmpMatcher.find()
738-
&& homeMatcher.find()) {
739-
return "etc";
759+
if (PROC_PATT.matcher(contents).find()
760+
&& ETC_PATT.matcher(contents).find()
761+
&& BOOT_PATT.matcher(contents).find()
762+
&& TMP_PATT.matcher(contents).find()
763+
&& HOME_PATT.matcher(contents).find()) {
764+
return NIX_DIR_EVIDENCE;
740765
}
741766

742767
return null;
743768
}
744769

745770
private static String matchWinDirectories(String contents) {
746-
if (contents.contains("Windows")
747-
&& Pattern.compile("Program\\sFiles").matcher(contents).find()) {
748-
return "Windows";
771+
if (contents.contains(WIN_DIR_EVIDENCE)
772+
&& Pattern.compile("Program\\sFiles", Pattern.CASE_INSENSITIVE)
773+
.matcher(contents)
774+
.find()) {
775+
return WIN_DIR_EVIDENCE;
749776
}
750777

751778
return null;

‎addOns/ascanrules/src/main/resources/org/zaproxy/zap/extension/ascanrules/resources/Messages.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ ascanrules.parametertamper.desc = Parameter manipulation caused an error page or
114114
ascanrules.parametertamper.name = Parameter Tampering
115115
ascanrules.parametertamper.soln = Identify the cause of the error and fix it. Do not trust client side input and enforce a tight check in the server side. Besides, catch the exception properly. Use a generic 500 error page for internal server error.
116116

117+
ascanrules.pathtraversal.info = While the evidence field indicates {0}, the rule actually checked that the response contains matches for all of the following: {1}.
117118
ascanrules.pathtraversal.name = Path Traversal
118119

119120
ascanrules.payloader.desc = Provides support for custom payloads in scan rules.

0 commit comments

Comments
 (0)