Java: fix 'matches' false branch

Author: Jami Cogswell

java/ql/lib/semmle/code/java/security/PathSanitizer.qll

@@ -497,9 +497,9 @@ private predicate isMatchesCall(StringMatchesCall matchesCall, Expr checkedExpr,
     target.getStringValue() = targetValue and
     checkedExpr = matchesCall.getQualifier()
   |
-    targetValue.matches(["[%]*", "[%]+", "[%]{%}"]) and
     (
       // Allow anything except `.`, '/', '\'
+      targetValue.matches(["[%]*", "[%]+", "[%]{%}"]) and
       (
         // Note: we do not account for when '.', '/', '\' are inside a character range
         not targetValue.matches("[%" + [".", "/", "\\\\\\\\"] + "%]%") and
@@ -512,9 +512,10 @@ private predicate isMatchesCall(StringMatchesCall matchesCall, Expr checkedExpr,
       branch = true
       or
       // Disallow `.`, '/', '\'
-      targetValue.matches("[%.%]%") and
-      targetValue.matches("[%/%]%") and
-      targetValue.matches("[%\\\\\\\\%]%") and
+      targetValue.matches([".*[%].*", ".+[%].+"]) and
+      targetValue.matches("%[%.%]%") and
+      targetValue.matches("%[%/%]%") and
+      targetValue.matches("%[%\\\\\\\\%]%") and
       not targetValue.matches("%[^%]%") and
       branch = false
     )

java/ql/test/library-tests/pathsanitizer/Test.java

@@ -684,16 +684,33 @@ public void directoryCharsSanitizer() throws Exception {
         // branch = false
         {
             String source = (String) source();
+            if (source.matches(".*[\\./\\\\].*")) {
+                sink(source); // $ hasTaintFlow
+            } else {
+                sink(source); // Safe
+            }
+        }
+        {
+            String source = (String) source();
+            if (source.matches(".+[\\./\\\\].+")) {
+                sink(source); // $ hasTaintFlow
+            } else {
+                sink(source); // Safe
+            }
+        }
+        {
+            String source = (String) source();
+            // does not match whole string
             if (source.matches("[\\./\\\\]+")) {
                 sink(source); // $ hasTaintFlow
             } else {
-                sink(source); // $ Safe
+                sink(source); // $ hasTaintFlow
             }
         }
         {
             String source = (String) source();
             // not a complete sanitizer since it doesn't protect against absolute path injection
-            if (source.matches("[\\.]+")) {
+            if (source.matches(".+[\\.].+")) {
                 sink(source); // $ hasTaintFlow
             } else {
                 sink(source); // $ hasTaintFlow