Skip to content

Commit 3b6b08e

Browse files
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]>
1 parent 3159ed2 commit 3b6b08e

File tree

4 files changed

+190
-50
lines changed

4 files changed

+190
-50
lines changed

addOns/ascanrules/CHANGELOG.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ All notable changes to this add-on will be documented in this file.
44
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
55

66
## Unreleased
7-
7+
### Changed
8+
- 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).
89

910
## [71] - 2025-03-04
1011
### Fixed

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

+74-47
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
8080
* Windows local file targets and detection pattern
8181
*/
8282
private static final ContentsMatcher WIN_PATTERN =
83-
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"));
83+
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"), Tech.Windows);
84+
private static final String WIN_DIR_EVIDENCE = "Windows";
8485
private static final String[] WIN_LOCAL_FILE_TARGETS = {
8586
// Absolute Windows file retrieval (we suppose C:\\)
8687
"c:/Windows/system.ini",
@@ -129,7 +130,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
129130
*/
130131
// Dot used to match 'x' or '!' (used in AIX)
131132
private static final ContentsMatcher NIX_PATTERN =
132-
new PatternContentsMatcher(Pattern.compile("root:.:0:0"));
133+
new PatternContentsMatcher(Pattern.compile("root:.:0:0"), Tech.Linux);
134+
private static final String NIX_DIR_EVIDENCE = "etc";
133135
private static final String[] NIX_LOCAL_FILE_TARGETS = {
134136
// Absolute file retrieval
135137
"/etc/passwd",
@@ -155,10 +157,9 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
155157
// ..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252Fetc%252Fpasswd%2500.jpg
156158
};
157159

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

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

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

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

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

347349
// Check if a there was a finding or the scan has been stopped
@@ -383,10 +385,9 @@ public void scan(HttpMessage msg, String param, String value) {
383385
// Check 3: Detect if this page is a directory browsing component
384386
if (inScope(Tech.Linux) || inScope(Tech.MacOS)) {
385387
for (int h = 0; h < nixDirCount; h++) {
386-
387388
// Check if a there was a finding or the scan has been stopped
388389
// if yes dispose resources and exit
389-
if (sendAndCheckPayload(param, NIX_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
390+
if (sendAndCheckPayload(param, NIX_LOCAL_DIR_TARGETS[h], NIX_DIR_MATCHER, 3)
390391
|| isStop()) {
391392
// Dispose all resources
392393
// Exit the scan rule
@@ -396,7 +397,7 @@ public void scan(HttpMessage msg, String param, String value) {
396397
}
397398
if (inScope(Tech.Windows)) {
398399
for (int h = 0; h < winDirCount; h++) {
399-
if (sendAndCheckPayload(param, WIN_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
400+
if (sendAndCheckPayload(param, WIN_LOCAL_DIR_TARGETS[h], WIN_DIR_MATCHER, 3)
400401
|| isStop()) {
401402
// Dispose all resources
402403
// Exit the scan rule
@@ -658,12 +659,23 @@ private AlertBuilder createUnmatchedAlert(String param, String attack) {
658659

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

669681
@Override
@@ -689,7 +701,7 @@ private static class PatternContentsMatcher implements ContentsMatcher {
689701

690702
private final Pattern pattern;
691703

692-
public PatternContentsMatcher(Pattern pattern) {
704+
public PatternContentsMatcher(Pattern pattern, Tech tech) {
693705
this.pattern = pattern;
694706
}
695707

@@ -705,46 +717,61 @@ public String match(String contents) {
705717

706718
private static class DirNamesContentsMatcher implements ContentsMatcher {
707719

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

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

741766
return null;
742767
}
743768

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

750777
return null;

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

+1
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)