diff --git a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll index 9072de7ce520..74eaa32684b6 100644 --- a/java/ql/lib/semmle/code/java/security/PathSanitizer.qll +++ b/java/ql/lib/semmle/code/java/security/PathSanitizer.qll @@ -352,3 +352,25 @@ private class FileGetNameSanitizer extends PathInjectionSanitizer { ) } } + +/** + * A sanitizer that considers the second argument to a `File` constructor safe + * if it is checked for `..` components (`PathTraversalGuard`) or if any internal + * `..` components are removed from it (`PathNormalizeSanitizer`). + */ +class FileConstructorSanitizer extends PathInjectionSanitizer { + FileConstructorSanitizer() { + exists(ConstructorCall constrCall, Argument arg, Expr guard | + constrCall.getConstructedType() instanceof TypeFile and + arg = constrCall.getArgument(1) and + ( + guard + .(PathTraversalGuard) + .controls(arg.getBasicBlock(), guard.(PathTraversalGuard).getBranch()) + or + TaintTracking::localExprTaint(guard.(PathNormalizeSanitizer), arg) + ) and + this.asExpr() = constrCall + ) + } +} diff --git a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java index 00447364bb38..f61e09e6f0b2 100644 --- a/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java +++ b/java/ql/test/query-tests/security/CWE-022/semmle/tests/TaintedPath.java @@ -86,4 +86,38 @@ public void sendUserFileGood4(Socket sock, String user) throws IOException { fileLine = fileReader.readLine(); } } + + public void sendUserFileGood5(Socket sock, String user) throws IOException { + BufferedReader filenameReader = + new BufferedReader(new InputStreamReader(sock.getInputStream(), "UTF-8")); + String filename = filenameReader.readLine(); + File f1 = new File("safe/file.txt"); + // GOOD: ensure that the path does not contain ".." and is used as the + // second argument to a `File` constructor + if (!filename.contains("..")) { + File f2 = new File(f1, filename); + f2.exists(); + + // Only sanitize `f2`; `filename` is still tainted + BufferedReader fileReader = new BufferedReader(new FileReader(filename)); // $ hasTaintFlow + } + } + + public void sendUserFileGood6(Socket sock, String user) throws IOException { + BufferedReader filenameReader = + new BufferedReader(new InputStreamReader(sock.getInputStream(), "UTF-8")); + String filename = filenameReader.readLine(); + File f1 = new File("safe/file.txt"); + + // GOOD: ensure that the path is normalized and is then used as the + // second argument to a `File` constructor + Path normalizedFilename = Paths.get(filename).normalize().toAbsolutePath(); + String normalizedFilenameStr = normalizedFilename.toString(); + File f2 = new File(f1, normalizedFilenameStr); + f2.exists(); + + // Only sanitize `f2`; `filename` and `normalizedFilenameStr` are still tainted + BufferedReader fileReader = new BufferedReader(new FileReader(filename)); // $ hasTaintFlow + BufferedReader fileReader2 = new BufferedReader(new FileReader(normalizedFilenameStr)); // $ hasTaintFlow + } }