Java: path sanitizer for replace, replaceAll, and matches

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

@@ -383,3 +383,92 @@ private class FileConstructorChildArgumentStep extends AdditionalTaintStep {
     )
   }
 }
+
+/**
+ * A complementary sanitizer that protects against path injection vulnerabilities
+ * by replacing all directory characters ('..', '/', and '\') with safe characters.
+ */
+private class ReplaceDirectoryCharactersSanitizer extends MethodCall {
+  ReplaceDirectoryCharactersSanitizer() {
+    exists(MethodCall mc |
+      // TODO: "java.lang.String.replace" as well
+      mc.getMethod().hasQualifiedName("java.lang", "String", "replaceAll") and
+      // TODO: unhardcode all of the below to handle more valid replacements and several calls
+      (
+        mc.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "\\.\\.|[/\\\\]"
+        or
+        exists(MethodCall mc2 |
+          mc2.getMethod().hasQualifiedName("java.lang", "String", "replaceAll") and
+          mc.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "\\." and
+          mc2.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "/"
+        )
+      ) and
+      // TODO: accept more replacement characters?
+      mc.getArgument(1).(CompileTimeConstantExpr).getStringValue() = ["", "_"] and
+      this = mc
+    )
+  }
+}
+
+/**
+ * A complementary guard that protects against path traversal by looking
+ * for patterns that exclude directory characters: `..`, '/', and '\'.
+ */
+private class DirectoryCharactersGuard extends PathGuard {
+  Expr checkedExpr;
+  boolean branch;
+
+  DirectoryCharactersGuard() {
+    exists(MethodCall mc, Method m, CompileTimeConstantExpr target |
+      m = mc.getMethod() and
+      m.getDeclaringType() instanceof TypeString and
+      m.hasName("matches") and
+      target = mc.getAnArgument() and
+      checkedExpr = mc.getQualifier() and
+      this = mc
+    |
+      // Allow anything except `.`, '/', '\'
+      (
+        not target.getStringValue().matches("%[^%]%") and
+        not target.getStringValue().matches("%" + ["\\.", "/", "\\\\"] + "%")
+        or
+        target.getStringValue().matches("%[^%" + ["\\.", "/", "\\\\"] + "%]%")
+      ) and
+      branch = true
+      or
+      // Disallow `.`, '/', '\'
+      (
+        not target.getStringValue().matches("%[^%" + ["\\.", "/", "\\\\"] + "%]%") and
+        // Assuming a regex containing line breaks is correctly matching line breaks in a string
+        target.getStringValue().matches("%" + ["\\.", "/", "\\\\"] + "%")
+      ) and
+      branch = false
+    )
+  }
+
+  override Expr getCheckedExpr() { result = checkedExpr }
+
+  boolean getBranch() { result = branch }
+}
+
+/**
+ * Holds if `g` is a guard that considers a path safe because it is checked to make
+ * sure it does not contain any directory characters: '..', '/', and '\'.
+ */
+private predicate directoryCharactersGuard(Guard g, Expr e, boolean branch) {
+  branch = g.(DirectoryCharactersGuard).getBranch() and
+  localTaintFlowToPathGuard(e, g)
+}
+
+/**
+ * A sanitizer that protects against path injection vulnerabilities
+ * by ensuring that the path does not contain any directory characters:
+ *  '..', '/', and '\'.
+ */
+private class DirectoryCharactersSanitizer extends PathInjectionSanitizer {
+  DirectoryCharactersSanitizer() {
+    this.asExpr() instanceof ReplaceDirectoryCharactersSanitizer or
+    this = DataFlow::BarrierGuard<directoryCharactersGuard/3>::getABarrierNode() or
+    this = ValidationMethod<directoryCharactersGuard/3>::getAValidatedNode()
+  }
+}

java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java

@@ -6,6 +6,7 @@
 import java.net.Socket;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.FileSystems;
 
 public class TaintedPath {
     public void sendUserFile(Socket sock, String user) throws IOException {
@@ -86,4 +87,51 @@ public void sendUserFileGood4(Socket sock, String user) throws IOException {
             fileLine = fileReader.readLine();
         }
     }
+
+    // TODO : New tests
+
+    public void sendUserFileGood5(Socket sock, String user) throws IOException {
+        BufferedReader filenameReader = new BufferedReader(new InputStreamReader(sock.getInputStream(), "UTF-8"));
+        // GOOD: remove all ".." sequences and path separators from the filename
+        String filename = filenameReader.readLine().replaceAll("\\.", "").replaceAll("/", "");
+        BufferedReader fileReader = new BufferedReader(new FileReader(filename)); // GOOD
+        String fileLine = fileReader.readLine();
+        while(fileLine != null) {
+            sock.getOutputStream().write(fileLine.getBytes());
+            fileLine = fileReader.readLine();
+        }
+    }
+
+    public void sendUserFileGood6(Socket sock, String user) throws IOException {
+        BufferedReader filenameReader = new BufferedReader(new InputStreamReader(sock.getInputStream(), "UTF-8"));
+        String filename = filenameReader.readLine();
+        // GOOD: remove all ".." sequences and path separators from the filename
+        filename = filename.replaceAll("\\.\\.|[/\\\\]", "");
+        BufferedReader fileReader = new BufferedReader(new FileReader(filename)); // GOOD
+        String fileLine = fileReader.readLine();
+        while(fileLine != null) {
+            sock.getOutputStream().write(fileLine.getBytes());
+            fileLine = fileReader.readLine();
+        }
+    }
+
+    public void sendUserFileGood7(Socket sock, String user) throws Exception {
+        BufferedReader filenameReader =
+                new BufferedReader(new InputStreamReader(sock.getInputStream(), "UTF-8"));
+        String filename = filenameReader.readLine();
+
+        // GOOD: ensure that that /, \ and .. cannot possibly be in the payload
+        if (filename.matches("[0-9a-fA-F]{20,}")) {
+            final Path pathObject = FileSystems.getDefault().getPath(filename); // summary now, see https://github.com/github/codeql/commit/19cb7adb6db17a3131b7db93482abc6a0d93ceff#diff-4b91db1bd2a19ab607f83fbe858f0ceffd942d1fb246739c731112367c865f88L8
+
+            BufferedReader fileReader = new BufferedReader(new FileReader(pathObject.toString())); // GOOD
+            String fileLine = fileReader.readLine();
+            while (fileLine != null) {
+                sock.getOutputStream().write(fileLine.getBytes());
+                fileLine = fileReader.readLine();
+            }
+        }
+
+    }
+
 }