Skip to content

Commit b9f665f

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. - 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 f2c298d commit b9f665f

File tree

4 files changed

+187
-45
lines changed

4 files changed

+187
-45
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
- Rules (as applicable) have been tagged in relation to HIPAA and PCI DSS.
1313
- The Cloud Metadata Potentially Exposed scan rules now has a CWE reference.
1414
- Scan rules which execute time based attacks now include the "TEST_TIMING" alert tag.
15+
- The Path Traversal scan rule now includes further details when directory matches are made (Issue 8379).
1516

1617
## [72] - 2025-06-20
1718
### Added

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

Lines changed: 74 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
8383
* Windows local file targets and detection pattern
8484
*/
8585
private static final ContentsMatcher WIN_PATTERN =
86-
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"));
86+
new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"), Tech.Windows);
87+
private static final String WIN_DIR_EVIDENCE = "Windows";
8788
private static final String[] WIN_LOCAL_FILE_TARGETS = {
8889
// Absolute Windows file retrieval (we suppose C:\\)
8990
"c:/Windows/system.ini",
@@ -132,7 +133,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
132133
*/
133134
// Dot used to match 'x' or '!' (used in AIX)
134135
private static final ContentsMatcher NIX_PATTERN =
135-
new PatternContentsMatcher(Pattern.compile("root:.:0:0"));
136+
new PatternContentsMatcher(Pattern.compile("root:.:0:0"), Tech.Linux);
137+
private static final String NIX_DIR_EVIDENCE = "etc";
136138
private static final String[] NIX_LOCAL_FILE_TARGETS = {
137139
// Absolute file retrieval
138140
"/etc/passwd",
@@ -158,10 +160,9 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
158160
// ..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252F..%252Fetc%252Fpasswd%2500.jpg
159161
};
160162

161-
/*
162-
* Windows/Unix/Linux/etc. local directory targets and detection pattern
163-
*/
164-
private static final ContentsMatcher DIR_PATTERN = new DirNamesContentsMatcher();
163+
private static final ContentsMatcher NIX_DIR_MATCHER = new DirNamesContentsMatcher(Tech.Linux);
164+
private static final ContentsMatcher WIN_DIR_MATCHER =
165+
new DirNamesContentsMatcher(Tech.Windows);
165166
private static final String[] WIN_LOCAL_DIR_TARGETS = {
166167
"c:/",
167168
"c:\\",
@@ -183,17 +184,19 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
183184
"/",
184185
"../../../../../../../../../../../../../../../../",
185186
"/../../../../../../../../../../../../../../../../",
186-
"file:///",
187+
"file:///"
187188
};
188189

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

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

198+
private static final List<String> DIR_EVIDENCE_LIST =
199+
List.of(NIX_DIR_EVIDENCE, WIN_DIR_EVIDENCE);
197200
/*
198201
* details of the vulnerability which we are attempting to find
199202
*/
@@ -389,7 +392,7 @@ public void scan(HttpMessage msg, String param, String value) {
389392

390393
// Check if a there was a finding or the scan has been stopped
391394
// if yes dispose resources and exit
392-
if (sendAndCheckPayload(param, NIX_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
395+
if (sendAndCheckPayload(param, NIX_LOCAL_DIR_TARGETS[h], NIX_DIR_MATCHER, 3)
393396
|| isStop()) {
394397
// Dispose all resources
395398
// Exit the scan rule
@@ -399,7 +402,7 @@ public void scan(HttpMessage msg, String param, String value) {
399402
}
400403
if (inScope(Tech.Windows)) {
401404
for (int h = 0; h < winDirCount; h++) {
402-
if (sendAndCheckPayload(param, WIN_LOCAL_DIR_TARGETS[h], DIR_PATTERN, 3)
405+
if (sendAndCheckPayload(param, WIN_LOCAL_DIR_TARGETS[h], WIN_DIR_MATCHER, 3)
403406
|| isStop()) {
404407
// Dispose all resources
405408
// Exit the scan rule
@@ -661,12 +664,23 @@ private AlertBuilder createUnmatchedAlert(String param, String attack) {
661664

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

672686
@Override
@@ -692,7 +706,7 @@ private static class PatternContentsMatcher implements ContentsMatcher {
692706

693707
private final Pattern pattern;
694708

695-
public PatternContentsMatcher(Pattern pattern) {
709+
public PatternContentsMatcher(Pattern pattern, Tech tech) {
696710
this.pattern = pattern;
697711
}
698712

@@ -708,46 +722,61 @@ public String match(String contents) {
708722

709723
private static class DirNamesContentsMatcher implements ContentsMatcher {
710724

725+
private static final String NIX_MATCHES =
726+
String.join(", ", List.of("proc", NIX_DIR_EVIDENCE, "boot", "tmp", "home"));
727+
private static final String WIN_MATCHES =
728+
String.join(", ", List.of(WIN_DIR_EVIDENCE, "Program Files"));
729+
private static final Pattern PROC_PATT =
730+
Pattern.compile(
731+
"(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
732+
private static final Pattern ETC_PATT =
733+
Pattern.compile(
734+
"(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
735+
private static final Pattern BOOT_PATT =
736+
Pattern.compile(
737+
"(?:^|\\W)boot(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
738+
private static final Pattern TMP_PATT =
739+
Pattern.compile(
740+
"(?:^|\\W)tmp(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
741+
private static final Pattern HOME_PATT =
742+
Pattern.compile(
743+
"(?:^|\\W)home(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
744+
745+
private Tech tech;
746+
747+
public DirNamesContentsMatcher(Tech tech) {
748+
this.tech = tech;
749+
}
750+
711751
@Override
712752
public String match(String contents) {
713-
String result = matchNixDirectories(contents);
714-
if (result != null) {
715-
return result;
753+
if (this.tech == Tech.Linux) {
754+
return matchNixDirectories(contents);
716755
}
717-
return matchWinDirectories(contents);
756+
if (this.tech == Tech.Windows) {
757+
return matchWinDirectories(contents);
758+
}
759+
return null;
718760
}
719761

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

744771
return null;
745772
}
746773

747774
private static String matchWinDirectories(String contents) {
748-
if (contents.contains("Windows")
749-
&& Pattern.compile("Program\\sFiles").matcher(contents).find()) {
750-
return "Windows";
775+
if (contents.contains(WIN_DIR_EVIDENCE)
776+
&& Pattern.compile("Program\\sFiles", Pattern.CASE_INSENSITIVE)
777+
.matcher(contents)
778+
.find()) {
779+
return WIN_DIR_EVIDENCE;
751780
}
752781

753782
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.

‎addOns/ascanrules/src/test/java/org/zaproxy/zap/extension/ascanrules/PathTraversalScanRuleUnitTest.java‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse;
2323
import static org.hamcrest.MatcherAssert.assertThat;
24+
import static org.hamcrest.Matchers.emptyString;
2425
import static org.hamcrest.Matchers.equalTo;
2526
import static org.hamcrest.Matchers.greaterThan;
2627
import static org.hamcrest.Matchers.hasSize;
@@ -31,10 +32,14 @@
3132
import fi.iki.elonen.NanoHTTPD.Response;
3233
import java.util.List;
3334
import java.util.Map;
35+
import java.util.stream.Stream;
3436
import org.apache.commons.lang3.ArrayUtils;
3537
import org.junit.jupiter.api.Test;
3638
import org.junit.jupiter.params.ParameterizedTest;
39+
import org.junit.jupiter.params.provider.Arguments;
40+
import org.junit.jupiter.params.provider.CsvSource;
3741
import org.junit.jupiter.params.provider.EnumSource;
42+
import org.junit.jupiter.params.provider.MethodSource;
3843
import org.junit.jupiter.params.provider.ValueSource;
3944
import org.parosproxy.paros.core.scanner.Alert;
4045
import org.parosproxy.paros.core.scanner.Plugin;
@@ -172,6 +177,13 @@ void shouldAlertIfAttackResponseListsWindowsDirectories() throws Exception {
172177
assertThat(alertsRaised.get(0).getAttack(), is(equalTo("c:/")));
173178
assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
174179
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
180+
assertThat(
181+
alertsRaised.get(0).getOtherInfo(),
182+
is(
183+
equalTo(
184+
"While the evidence field indicates Windows, the rule actually "
185+
+ "checked that the response contains matches for all of the "
186+
+ "following: Windows, Program Files.")));
175187
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
176188
}
177189

@@ -190,6 +202,13 @@ void shouldAlertIfAttackResponseListsLinuxDirectories() throws Exception {
190202
assertThat(alertsRaised.get(0).getAttack(), is(equalTo("/")));
191203
assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
192204
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
205+
assertThat(
206+
alertsRaised.get(0).getOtherInfo(),
207+
is(
208+
equalTo(
209+
"While the evidence field indicates etc, the rule actually "
210+
+ "checked that the response contains matches for all of the "
211+
+ "following: proc, etc, boot, tmp, home.")));
193212
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
194213
}
195214

@@ -208,6 +227,13 @@ void shouldAlertIfAttackResponseListsLinuxDirectoriesInPlainText() throws Except
208227
assertThat(alertsRaised.get(0).getAttack(), is(equalTo("/")));
209228
assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
210229
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
230+
assertThat(
231+
alertsRaised.get(0).getOtherInfo(),
232+
is(
233+
equalTo(
234+
"While the evidence field indicates etc, the rule actually "
235+
+ "checked that the response contains matches for all of the "
236+
+ "following: proc, etc, boot, tmp, home.")));
211237
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
212238
}
213239

@@ -281,6 +307,7 @@ void shouldRaiseAlertIfResponseHasPasswdFileContentAndPayloadIsNullByteBased()
281307
// Then
282308
assertThat(alertsRaised, hasSize(1));
283309
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-2")));
310+
assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
284311
}
285312

286313
@Test
@@ -297,6 +324,7 @@ void shouldRaiseAlertIfResponseHasSystemINIFileContentAndPayloadIsNullByteBased(
297324
// Then
298325
assertThat(alertsRaised, hasSize(1));
299326
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-1")));
327+
assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
300328
}
301329

302330
@ParameterizedTest
@@ -317,6 +345,7 @@ void shouldAlertOnCheckFiveBelowHighThresholdUnderValidConditions(AlertThreshold
317345
assertThat(alertsRaised, hasSize(1));
318346
assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW)));
319347
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-5")));
348+
assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
320349
}
321350

322351
@Test
@@ -365,6 +394,88 @@ void shouldNotAlertOnCheckFiveAtLowThresholdUnderInvalidConditions(String errorT
365394
assertThat(alertsRaised, hasSize(0));
366395
}
367396

397+
@ParameterizedTest
398+
@CsvSource({
399+
// Windows will always happen before ignoring Linux, if only Linux pre-check matches
400+
"foo etc root tmp bin boot dev home mnt opt proc bar, 13",
401+
"foo Program Files Users Windows bar, 13",
402+
"foo etc root tmp bin boot dev home mnt opt proc Program Files Users Windows bar, 11"
403+
})
404+
void shouldSkipDirChecksIfResponseHasDirEvidenceToStartWith(String content, int expected)
405+
throws Exception {
406+
// Given
407+
HttpMessage msg = getHttpMessage("/?p=v");
408+
msg.setResponseBody(content);
409+
rule.init(msg, parent);
410+
// When
411+
rule.scan();
412+
// Then
413+
assertThat(httpMessagesSent, hasSize(equalTo(expected)));
414+
}
415+
416+
private static Stream<Arguments> handlersProvider() {
417+
return Stream.of(
418+
// List Linux dirs on attack, but skip windiws payloads since it has windows
419+
// evidence to start with
420+
Arguments.of(
421+
new ListLinuxDirsOnAttack("/", "p", "/"),
422+
"foo Program Files Users Windows bar",
423+
List.of(
424+
"c:/",
425+
"c:\\",
426+
"..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\",
427+
"\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\..\\",
428+
"file:///c:/",
429+
"file:///c:\\",
430+
"file:\\\\\\c:\\",
431+
"file:\\\\\\c:/",
432+
"file:\\\\\\",
433+
"d:\\",
434+
"d:/",
435+
"file:///d:/",
436+
"file:///d:\\",
437+
"file:\\\\\\d:\\",
438+
"file:\\\\\\d:/")),
439+
// List win dirs on attack, but skip linux payloads since it has linux
440+
// evidence to start with
441+
Arguments.of(
442+
new ListWinDirsOnAttack("/", "p", "c:/"),
443+
"foo etc root tmp bin boot dev home mnt opt proc bar",
444+
List.of(
445+
"/",
446+
"../../../../../../../../../../../../../../../../",
447+
"/../../../../../../../../../../../../../../../../",
448+
"file:///")));
449+
}
450+
451+
@ParameterizedTest
452+
@MethodSource("handlersProvider")
453+
void shouldNotSkipIfGoodCandidateForOnlyOneTech(
454+
NanoServerHandler handler, String baseBody, List<String> skippedPayloads)
455+
throws Exception {
456+
// Given
457+
nano.addHandler(handler);
458+
HttpMessage msg = getHttpMessage("/?p=v");
459+
msg.setResponseBody(baseBody);
460+
rule.init(msg, parent);
461+
// When
462+
rule.scan();
463+
// Then
464+
assertSkippedPayloadsNotUsed(httpMessagesSent, skippedPayloads);
465+
assertThat(alertsRaised, hasSize(equalTo(1)));
466+
assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
467+
}
468+
469+
private static void assertSkippedPayloadsNotUsed(
470+
List<HttpMessage> httpMessagesSent, List<String> skippedPayloads) {
471+
for (HttpMessage m : httpMessagesSent) {
472+
String paramVal = m.getUrlParams().first().getValue();
473+
for (String payload : skippedPayloads) {
474+
assertThat(paramVal.equals(payload), is(equalTo(false)));
475+
}
476+
}
477+
}
478+
368479
private abstract static class ListDirsOnAttack extends NanoServerHandler {
369480

370481
private final String param;

0 commit comments

Comments
 (0)