Java: fix replacement char check and add tests

Author: Jami Cogswell

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

@@ -419,7 +419,8 @@ private predicate replacesDirectoryCharactersWithSingleReplaceAll(
 ) {
   exists(CompileTimeConstantExpr target, string targetValue |
     isReplaceAllTarget(replaceAllCall, target) and
-    target.getStringValue() = targetValue
+    target.getStringValue() = targetValue and
+    replaceAllCall.getArgument(1).(CompileTimeConstantExpr).getStringValue() = getAReplacementChar()
   |
     not targetValue.matches("%[^%]%") and
     targetValue.matches("[%.%]") and
@@ -460,6 +461,7 @@ private predicate replacesDirectoryCharactersWithDoubleReplaceOrReplaceAll(
     rc2.getQualifier() = rc1 and
     target1.getStringValue() = targetValue1 and
     target2.getStringValue() = targetValue2 and
+    rc1.getArgument(1).(CompileTimeConstantExpr).getStringValue() = getAReplacementChar() and
     rc2.getArgument(1).(CompileTimeConstantExpr).getStringValue() = getAReplacementChar() and
     // make sure the calls replace different characters
     targetValue2 != targetValue1 and

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

@@ -716,14 +716,20 @@ public void directoryCharsSanitizer() throws Exception {
         }
         {
             String source = (String) source();
-            source = source.replaceAll("\\.|[/\\\\]", "");
+            source = source.replaceAll("\\.|[/\\\\]", "-");
             sink(source); // Safe
         }
         {
             String source = (String) source();
-            source = source.replaceAll("[.][.]|[/\\\\]", "");
+            source = source.replaceAll("[.][.]|[/\\\\]", "_");
             sink(source); // Safe
         }
+        {
+            String source = (String) source();
+            // test a not-accepted replacement character
+            source = source.replaceAll("[.][.]|[/\\\\]", "/");
+            sink(source); // $ hasTaintFlow
+        }
         {
             String source = (String) source();
             source = source.replaceAll(".|[/\\\\]", "");
@@ -761,6 +767,24 @@ public void directoryCharsSanitizer() throws Exception {
             source = source.replaceAll("\\.", "").replaceAll("/", "");
             sink(source); // Safe
         }
+        {
+            String source = (String) source();
+            // test a not-accepted replacement character in each call
+            source = source.replaceAll("\\.", "/").replaceAll("/", ".");
+            sink(source); // $ hasTaintFlow
+        }
+        {
+            String source = (String) source();
+            // test a not-accepted replacement character in first call
+            source = source.replaceAll("\\.", "/").replaceAll("/", "-");
+            sink(source); // $ hasTaintFlow
+        }
+        {
+            String source = (String) source();
+            // test a not-accepted replacement character in second call
+            source = source.replaceAll("\\.", "_").replaceAll("/", ".");
+            sink(source); // $ hasTaintFlow
+        }
         {
             String source = (String) source();
             source = source.replaceAll("\\.", "").replaceAll("/", "").replaceAll("\\\\", "");