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]>

Author: kingthorin

addOns/ascanrules/CHANGELOG.md

@@ -4,7 +4,8 @@ All notable changes to this add-on will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
-
+### Changed
+- 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).
 
 ## [71] - 2025-03-04
 ### Fixed

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

@@ -81,6 +81,7 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
      */
     private static final ContentsMatcher WIN_PATTERN =
             new PatternContentsMatcher(Pattern.compile("\\[drivers\\]"));
+    private static final String WIN_DIR_EVIDENCE = "Windows";
     private static final String[] WIN_LOCAL_FILE_TARGETS = {
         // Absolute Windows file retrieval (we suppose C:\\)
         "c:/Windows/system.ini",
@@ -130,6 +131,7 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
     // Dot used to match 'x' or '!' (used in AIX)
     private static final ContentsMatcher NIX_PATTERN =
             new PatternContentsMatcher(Pattern.compile("root:.:0:0"));
+    private static final String NIX_DIR_EVIDENCE = "etc";
     private static final String[] NIX_LOCAL_FILE_TARGETS = {
         // Absolute file retrieval
         "/etc/passwd",
@@ -191,6 +193,8 @@ public class PathTraversalScanRule extends AbstractAppParamPlugin
      */
     private static final String[] LOCAL_FILE_RELATIVE_PREFIXES = {"", "/", "\\"};
 
+    private static final List<String> DIR_EVIDENCE_LIST =
+            List.of(NIX_DIR_EVIDENCE, WIN_DIR_EVIDENCE);
     /*
      * details of the vulnerability which we are attempting to find
      */
@@ -303,6 +307,9 @@ public void scan(HttpMessage msg, String param, String value) {
 
             // Check 1: Start detection for Windows patterns
             if (inScope(Tech.Windows)) {
+                if (!isGoodCandidate(getBaseMsg(), Tech.Windows)) {
+                    return;
+                }
 
                 for (int h = 0; h < winCount; h++) {
 
@@ -341,7 +348,9 @@ public void scan(HttpMessage msg, String param, String value) {
 
             // Check 2: Start detection for *NIX patterns
             if (inScope(Tech.Linux) || inScope(Tech.MacOS)) {
-
+                if (!isGoodCandidate(getBaseMsg(), Tech.Linux)) {
+                    return;
+                }
                 for (int h = 0; h < nixCount; h++) {
 
                     // Check if a there was a finding or the scan has been stopped
@@ -569,6 +578,18 @@ public void scan(HttpMessage msg, String param, String value) {
         }
     }
 
+    private static boolean isGoodCandidate(HttpMessage msg, Tech targetTech) {
+        if (targetTech == Tech.Windows) {
+            return DirNamesContentsMatcher.matchWinDirectories(msg.getResponseBody().toString())
+                    == null;
+        }
+        if (targetTech == Tech.Linux) {
+            return DirNamesContentsMatcher.matchNixDirectories(msg.getResponseBody().toString())
+                    == null;
+        }
+        return false;
+    }
+
     private boolean sendAndCheckPayload(
             String param, String newValue, ContentsMatcher contentsMatcher, int check)
             throws IOException {
@@ -658,12 +679,23 @@ private AlertBuilder createUnmatchedAlert(String param, String attack) {
 
     private AlertBuilder createMatchedAlert(
             String param, String attack, String evidence, int check) {
-        return newAlert()
-                .setConfidence(Alert.CONFIDENCE_MEDIUM)
-                .setParam(param)
-                .setAttack(attack)
-                .setEvidence(evidence)
-                .setAlertRef(getId() + "-" + check);
+        AlertBuilder builder =
+                newAlert()
+                        .setConfidence(Alert.CONFIDENCE_MEDIUM)
+                        .setParam(param)
+                        .setAttack(attack)
+                        .setEvidence(evidence)
+                        .setAlertRef(getId() + "-" + check);
+        if (DIR_EVIDENCE_LIST.contains(evidence)) {
+            builder.setOtherInfo(
+                    Constant.messages.getString(
+                            MESSAGE_PREFIX + "info",
+                            evidence,
+                            evidence.equals(WIN_DIR_EVIDENCE)
+                                    ? DirNamesContentsMatcher.WIN_MATCHES
+                                    : DirNamesContentsMatcher.NIX_MATCHES));
+        }
+        return builder;
     }
 
     @Override
@@ -705,6 +737,26 @@ public String match(String contents) {
 
     private static class DirNamesContentsMatcher implements ContentsMatcher {
 
+        private static final String NIX_MATCHES =
+                String.join(", ", List.of("proc", NIX_DIR_EVIDENCE, "boot", "tmp", "home"));
+        private static final String WIN_MATCHES =
+                String.join(", ", List.of(WIN_DIR_EVIDENCE, "Program Files"));
+        private static final Pattern PROC_PATT =
+                Pattern.compile(
+                        "(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
+        private static final Pattern ETC_PATT =
+                Pattern.compile(
+                        "(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
+        private static final Pattern BOOT_PATT =
+                Pattern.compile(
+                        "(?:^|\\W)boot(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
+        private static final Pattern TMP_PATT =
+                Pattern.compile(
+                        "(?:^|\\W)tmp(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
+        private static final Pattern HOME_PATT =
+                Pattern.compile(
+                        "(?:^|\\W)home(?:\\W|$)", Pattern.CASE_INSENSITIVE | Pattern.MULTILINE);
+
         @Override
         public String match(String contents) {
             String result = matchNixDirectories(contents);
@@ -715,36 +767,21 @@ public String match(String contents) {
         }
 
         private static String matchNixDirectories(String contents) {
-            Pattern procPattern =
-                    Pattern.compile("(?:^|\\W)proc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
-            Pattern etcPattern = Pattern.compile("(?:^|\\W)etc(?:\\W|$)", Pattern.CASE_INSENSITIVE);
-            Pattern bootPattern =
-                    Pattern.compile("(?:^|\\W)boot(?:\\W|$)", Pattern.CASE_INSENSITIVE);
-            Pattern tmpPattern = Pattern.compile("(?:^|\\W)tmp(?:\\W|$)", Pattern.CASE_INSENSITIVE);
-            Pattern homePattern =
-                    Pattern.compile("(?:^|\\W)home(?:\\W|$)", Pattern.CASE_INSENSITIVE);
-
-            Matcher procMatcher = procPattern.matcher(contents);
-            Matcher etcMatcher = etcPattern.matcher(contents);
-            Matcher bootMatcher = bootPattern.matcher(contents);
-            Matcher tmpMatcher = tmpPattern.matcher(contents);
-            Matcher homeMatcher = homePattern.matcher(contents);
-
-            if (procMatcher.find()
-                    && etcMatcher.find()
-                    && bootMatcher.find()
-                    && tmpMatcher.find()
-                    && homeMatcher.find()) {
-                return "etc";
+            if (PROC_PATT.matcher(contents).find()
+                    && ETC_PATT.matcher(contents).find()
+                    && BOOT_PATT.matcher(contents).find()
+                    && TMP_PATT.matcher(contents).find()
+                    && HOME_PATT.matcher(contents).find()) {
+                return NIX_DIR_EVIDENCE;
             }
 
             return null;
         }
 
         private static String matchWinDirectories(String contents) {
-            if (contents.contains("Windows")
+            if (contents.contains(WIN_DIR_EVIDENCE)
                     && Pattern.compile("Program\\sFiles").matcher(contents).find()) {
-                return "Windows";
+                return WIN_DIR_EVIDENCE;
             }
 
             return null;

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

@@ -114,6 +114,7 @@ ascanrules.parametertamper.desc = Parameter manipulation caused an error page or
 ascanrules.parametertamper.name = Parameter Tampering
 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.
 
+ascanrules.pathtraversal.info = While the evidence field indicates {0}, the rule actually checked that the response contains matches for all of the following: {1}.
 ascanrules.pathtraversal.name = Path Traversal
 
 ascanrules.payloader.desc = Provides support for custom payloads in scan rules.

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

@@ -21,6 +21,7 @@
 
 import static fi.iki.elonen.NanoHTTPD.newFixedLengthResponse;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.emptyString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
 import static org.hamcrest.Matchers.hasSize;
@@ -34,6 +35,7 @@
 import org.apache.commons.lang3.ArrayUtils;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.EnumSource;
 import org.junit.jupiter.params.provider.ValueSource;
 import org.parosproxy.paros.core.scanner.Alert;
@@ -169,6 +171,13 @@ void shouldAlertIfAttackResponseListsWindowsDirectories() throws Exception {
         assertThat(alertsRaised.get(0).getAttack(), is(equalTo("c:/")));
         assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
         assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
+        assertThat(
+                alertsRaised.get(0).getOtherInfo(),
+                is(
+                        equalTo(
+                                "While the evidence field indicates Windows, the rule actually "
+                                        + "checked that the response contains matches for all of the "
+                                        + "following: Windows, Program Files.")));
         assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
     }
 
@@ -187,6 +196,13 @@ void shouldAlertIfAttackResponseListsLinuxDirectories() throws Exception {
         assertThat(alertsRaised.get(0).getAttack(), is(equalTo("/")));
         assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
         assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
+        assertThat(
+                alertsRaised.get(0).getOtherInfo(),
+                is(
+                        equalTo(
+                                "While the evidence field indicates etc, the rule actually "
+                                        + "checked that the response contains matches for all of the "
+                                        + "following: proc, etc, boot, tmp, home.")));
         assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
     }
 
@@ -205,6 +221,13 @@ void shouldAlertIfAttackResponseListsLinuxDirectoriesInPlainText() throws Except
         assertThat(alertsRaised.get(0).getAttack(), is(equalTo("/")));
         assertThat(alertsRaised.get(0).getRisk(), is(equalTo(Alert.RISK_HIGH)));
         assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_MEDIUM)));
+        assertThat(
+                alertsRaised.get(0).getOtherInfo(),
+                is(
+                        equalTo(
+                                "While the evidence field indicates etc, the rule actually "
+                                        + "checked that the response contains matches for all of the "
+                                        + "following: proc, etc, boot, tmp, home.")));
         assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-3")));
     }
 
@@ -278,6 +301,7 @@ void shouldRaiseAlertIfResponseHasPasswdFileContentAndPayloadIsNullByteBased()
         // Then
         assertThat(alertsRaised, hasSize(1));
         assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-2")));
+        assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
     }
 
     @Test
@@ -294,6 +318,7 @@ void shouldRaiseAlertIfResponseHasSystemINIFileContentAndPayloadIsNullByteBased(
         // Then
         assertThat(alertsRaised, hasSize(1));
         assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-1")));
+        assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
     }
 
     @ParameterizedTest
@@ -314,6 +339,7 @@ void shouldAlertOnCheckFiveBelowHighThresholdUnderValidConditions(AlertThreshold
         assertThat(alertsRaised, hasSize(1));
         assertThat(alertsRaised.get(0).getConfidence(), is(equalTo(Alert.CONFIDENCE_LOW)));
         assertThat(alertsRaised.get(0).getAlertRef(), is(equalTo("6-5")));
+        assertThat(alertsRaised.get(0).getOtherInfo(), is(emptyString()));
     }
 
     @Test
@@ -362,6 +388,24 @@ void shouldNotAlertOnCheckFiveAtLowThresholdUnderInvalidConditions(String errorT
         assertThat(alertsRaised, hasSize(0));
     }
 
+    @ParameterizedTest
+    @CsvSource({
+        // Windows will always happen before ignoring Linux, if only Linux pre-check matches
+        "foo etc root tmp bin boot dev home mnt opt proc bar, 4",
+        "foo Program Files Users Windows bar, 0",
+        "foo etc root tmp bin boot dev home mnt opt proc Program Files Users Windows bar, 0"
+    })
+    void shouldSkipIfResponseHasEvidenceToStartWith(String content, int expected) throws Exception {
+        // Given
+        HttpMessage msg = getHttpMessage("/?p=v");
+        msg.setResponseBody(content);
+        rule.init(msg, parent);
+        // When
+        rule.scan();
+        // Then
+        assertThat(httpMessagesSent, hasSize(equalTo(expected)));
+    }
+
     private abstract static class ListDirsOnAttack extends NanoServerHandler {
 
         private final String param;