Skip to content

Commit 3567772

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 <kingthorin@users.noreply.github.com>
1 parent 3159ed2 commit 3567772

File tree

4 files changed

+171
-39
lines changed

4 files changed

+171
-39
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

+80-36
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
8181
*/
8282
private static final ContentsMatcher WIN_PATTERN =
8383
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"));
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",
@@ -130,6 +131,7 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
130131
// Dot used to match 'x' or '!' (used in AIX)
131132
private static final ContentsMatcher NIX_PATTERN =
132133
new PatternContentsMatcher(Pattern.compile("root:.:0:0"));
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",
@@ -159,7 +161,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
159161
* Windows/Unix/Linux/etc. local directory targets and detection pattern
160162
*/
161163
private static final ContentsMatcher DIR_PATTERN = new DirNamesContentsMatcher();
162-
private static final String[] WIN_LOCAL_DIR_TARGETS = {
164+
/* Increased visibility purely for testing purposes */
165+
public static final String[] WIN_LOCAL_DIR_TARGETS = {
163166
"c:/",
164167
"c:\\",
165168
"..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\",
@@ -176,7 +179,7 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
176179
"file:\\\\\\d:\\",
177180
"file:\\\\\\d:/"
178181
};
179-
private static final String[] NIX_LOCAL_DIR_TARGETS = {
182+
public static final String[] NIX_LOCAL_DIR_TARGETS = {
180183
"/",
181184
"../../../../../../../../../../../../../../../../",
182185
"/../../../../../../../../../../../../../../../../",
@@ -191,6 +194,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
191194
*/
192195
private static final String[] LOCAL_FILE_RELATIVE_PREFIXES = {"", "/", "\\"};
193196

197+
private static final List<String> DIR_EVIDENCE_LIST =
198+
List.of(NIX_DIR_EVIDENCE, WIN_DIR_EVIDENCE);
194199
/*
195200
* details of the vulnerability which we are attempting to find
196201
*/
@@ -341,7 +346,6 @@ public void scan(HttpMessage msg, String param, String value) {
341346

342347
// Check 2: Start detection for *NIX patterns
343348
if (inScope(Tech.Linux) || inScope(Tech.MacOS)) {
344-
345349
for (int h = 0; h < nixCount; h++) {
346350

347351
// Check if a there was a finding or the scan has been stopped
@@ -381,9 +385,13 @@ public void scan(HttpMessage msg, String param, String value) {
381385
}
382386

383387
// Check 3: Detect if this page is a directory browsing component
384-
if (inScope(Tech.Linux) || inScope(Tech.MacOS)) {
388+
// System.out.println(
389+
// "Not Linux evidence? " + !isEvidenceInResponse(getBaseMsg(),
390+
// Tech.Linux));
391+
// System.out.println("Base: " + getBaseMsg().getResponseBody().toString());
392+
if (inScope(Tech.Linux)
393+
|| inScope(Tech.MacOS) && !isEvidenceInResponse(getBaseMsg(), Tech.Linux)) {
385394
for (int h = 0; h < nixDirCount; h++) {
386-
387395
// Check if a there was a finding or the scan has been stopped
388396
// if yes dispose resources and exit
389397
if (sendAndCheckPayload(param, NIX_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
@@ -394,7 +402,11 @@ public void scan(HttpMessage msg, String param, String value) {
394402
}
395403
}
396404
}
397-
if (inScope(Tech.Windows)) {
405+
// System.out.println(
406+
// "Not Windows evidence? " + !isEvidenceInResponse(getBaseMsg(),
407+
// Tech.Windows));
408+
// System.out.println("Base: " + getBaseMsg().getResponseBody().toString());
409+
if (inScope(Tech.Windows) && !isEvidenceInResponse(getBaseMsg(), Tech.Windows)) {
398410
for (int h = 0; h < winDirCount; h++) {
399411
if (sendAndCheckPayload(param, WIN_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
400412
|| isStop()) {
@@ -569,6 +581,19 @@ public void scan(HttpMessage msg, String param, String value) {
569581
}
570582
}
571583

584+
private static boolean isEvidenceInResponse(HttpMessage msg, Tech targetTech) {
585+
// It is a good candidate if it doesn't have evidence to start with
586+
if (targetTech == Tech.Windows) {
587+
return DirNamesContentsMatcher.matchWinDirectories(msg.getResponseBody().toString())
588+
!= null;
589+
}
590+
if (targetTech == Tech.Linux) {
591+
return DirNamesContentsMatcher.matchNixDirectories(msg.getResponseBody().toString())
592+
!= null;
593+
}
594+
return false;
595+
}
596+
572597
private boolean sendAndCheckPayload(
573598
String param, String newValue, ContentsMatcher contentsMatcher, int check)
574599
throws IOException {
@@ -607,6 +632,7 @@ private boolean sendAndCheckPayload(
607632
return false; // Something went wrong, no point continuing
608633
}
609634

635+
// System.out.println("Tested body: " + msg.getResponseBody().toString());
610636
// does it match the pattern specified for that file name?
611637
String match = contentsMatcher.match(getContentsToMatch(msg));
612638

@@ -658,12 +684,23 @@ private AlertBuilder createUnmatchedAlert(String param, String attack) {
658684

659685
private AlertBuilder createMatchedAlert(
660686
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);
687+
AlertBuilder builder =
688+
newAlert()
689+
.setConfidence(Alert.CONFIDENCE_MEDIUM)
690+
.setParam(param)
691+
.setAttack(attack)
692+
.setEvidence(evidence)
693+
.setAlertRef(getId() + "-" + check);
694+
if (DIR_EVIDENCE_LIST.contains(evidence)) {
695+
builder.setOtherInfo(
696+
Constant.messages.getString(
697+
MESSAGE_PREFIX + "info",
698+
evidence,
699+
evidence.equals(WIN_DIR_EVIDENCE)
700+
? DirNamesContentsMatcher.WIN_MATCHES
701+
: DirNamesContentsMatcher.NIX_MATCHES));
702+
}
703+
return builder;
667704
}
668705

669706
@Override
@@ -705,6 +742,26 @@ public String match(String contents) {
705742

706743
private static class DirNamesContentsMatcher implements ContentsMatcher {
707744

745+
private static final String NIX_MATCHES =
746+
String.join(", ", List.of("proc", NIX_DIR_EVIDENCE, "boot", "tmp", "home"));
747+
private static final String WIN_MATCHES =
748+
String.join(", ", List.of(WIN_DIR_EVIDENCE, "Program Files"));
749+
private static final Pattern PROC_PATT =
750+
Pattern.compile(
751+
"(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
752+
private static final Pattern ETC_PATT =
753+
Pattern.compile(
754+
"(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
755+
private static final Pattern BOOT_PATT =
756+
Pattern.compile(
757+
"(?:^|\\W)boot(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
758+
private static final Pattern TMP_PATT =
759+
Pattern.compile(
760+
"(?:^|\\W)tmp(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
761+
private static final Pattern HOME_PATT =
762+
Pattern.compile(
763+
"(?:^|\\W)home(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
764+
708765
@Override
709766
public String match(String contents) {
710767
String result = matchNixDirectories(contents);
@@ -715,36 +772,23 @@ public String match(String contents) {
715772
}
716773

717774
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";
775+
if (PROC_PATT.matcher(contents).find()
776+
&& ETC_PATT.matcher(contents).find()
777+
&& BOOT_PATT.matcher(contents).find()
778+
&& TMP_PATT.matcher(contents).find()
779+
&& HOME_PATT.matcher(contents).find()) {
780+
return NIX_DIR_EVIDENCE;
739781
}
740782

741783
return null;
742784
}
743785

744786
private static String matchWinDirectories(String contents) {
745-
if (contents.contains("Windows")
746-
&& Pattern.compile("Program\\sFiles").matcher(contents).find()) {
747-
return "Windows";
787+
if (contents.contains(WIN_DIR_EVIDENCE)
788+
&& Pattern.compile("Program\\sFiles", Pattern.CASE_INSENSITIVE)
789+
.matcher(contents)
790+
.find()) {
791+
return WIN_DIR_EVIDENCE;
748792
}
749793

750794
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)