Java: fix handling of '^' and '.' in matches case

github · Feb 27, 2025 · ca19531 · ca19531
1 parent 6512d5f
commit ca19531
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 3 deletions.
diff --git a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll
@@ -493,13 +493,20 @@ private class DirectoryCharactersGuard extends PathGuard {
       (
         // Allow anything except `.`, '/', '\'
         // Note: we do not account for when '.', '/', '\' are inside a character range
-        not target.getStringValue().matches("[%" + ["\\.", "/", "\\\\"] + "%]%") and
+        (
+          not target.getStringValue().matches("[%" + [".", "/", "\\\\"] + "%]%") and
+          not target.getStringValue().matches("%[^%]%")
+          or
+          target.getStringValue().matches("[^%.%]%") and
+          target.getStringValue().matches("[^%/%]%") and
+          target.getStringValue().matches("[^%\\\\%]%")
+        ) and
         branch = true
         or
-        // Disallow `.`, '/', '\'
-        target.getStringValue().matches("[%\\.%]%") and
+        target.getStringValue().matches("[%.%]%") and
         target.getStringValue().matches("[%/%]%") and
         target.getStringValue().matches("[%\\\\%]%") and
+        not target.getStringValue().matches("%[^%]%") and
         branch = false
       )
     )

diff --git a/java/ql/test/library-tests/pathsanitizer/Test.java b/java/ql/test/library-tests/pathsanitizer/Test.java
@@ -631,6 +631,24 @@ public void directoryCharsSanitizer() throws Exception {
                 sink(source); // $ hasTaintFlow
             }
         }
+        {
+            String source = (String) source();
+            // exclude '.', '/', '\'
+            if (source.matches("[^0-9./\\\\a-f]{20,}")) {
+                sink(source); // Safe
+            } else {
+                sink(source); // $ hasTaintFlow
+            }
+        }
+        {
+            String source = (String) source();
+            // '.' is not excluded
+            if (source.matches("[^0-9/\\\\a-f]{20,}")) {
+                sink(source); // $ hasTaintFlow
+            } else {
+                sink(source); // $ hasTaintFlow
+            }
+        }
         // branch = false
         {
             String source = (String) source();