Skip to content

Commit 4dc7562

Browse files
Jami CogswellJami Cogswell
Jami Cogswell
authored and
Jami Cogswell
committed
Java: add FileConstructorSanitizer and tests
1 parent 90faab4 commit 4dc7562

File tree

2 files changed

+56
-0
lines changed

2 files changed

+56
-0
lines changed

Diff for: java/ql/lib/semmle/code/java/security/PathSanitizer.qll

+22
Original file line numberDiff line numberDiff line change
@@ -352,3 +352,25 @@ private class FileGetNameSanitizer extends PathInjectionSanitizer {
352352
)
353353
}
354354
}
355+
356+
/**
357+
* A sanitizer that considers the second argument to a `File` constructor safe
358+
* if it is checked for `..` components (`PathTraversalGuard`) or if any internal
359+
* `..` components are removed from it (`PathNormalizeSanitizer`).
360+
*/
361+
class FileConstructorSanitizer extends PathInjectionSanitizer {
362+
FileConstructorSanitizer() {
363+
exists(ConstructorCall constrCall, Argument arg, Expr guard |
364+
constrCall.getConstructedType() instanceof TypeFile and
365+
arg = constrCall.getArgument(1) and
366+
(
367+
guard
368+
.(PathTraversalGuard)
369+
.controls(arg.getBasicBlock(), guard.(PathTraversalGuard).getBranch())
370+
or
371+
TaintTracking::localExprTaint(guard.(PathNormalizeSanitizer), arg)
372+
) and
373+
this.asExpr() = constrCall
374+
)
375+
}
376+
}

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

+34
Original file line numberDiff line numberDiff line change
@@ -86,4 +86,38 @@ public void sendUserFileGood4(Socket sock, String user) throws IOException {
8686
fileLine = fileReader.readLine();
8787
}
8888
}
89+
90+
public void sendUserFileGood5(Socket sock, String user) throws IOException {
91+
BufferedReader filenameReader =
92+
new BufferedReader(new InputStreamReader(sock.getInputStream(), "UTF-8"));
93+
String filename = filenameReader.readLine();
94+
File f1 = new File("safe/file.txt");
95+
// GOOD: ensure that the path does not contain ".." and is used as the
96+
// second argument to a `File` constructor
97+
if (!filename.contains("..")) {
98+
File f2 = new File(f1, filename);
99+
f2.exists();
100+
101+
// Only sanitize `f2`; `filename` is still tainted
102+
BufferedReader fileReader = new BufferedReader(new FileReader(filename)); // $ hasTaintFlow
103+
}
104+
}
105+
106+
public void sendUserFileGood6(Socket sock, String user) throws IOException {
107+
BufferedReader filenameReader =
108+
new BufferedReader(new InputStreamReader(sock.getInputStream(), "UTF-8"));
109+
String filename = filenameReader.readLine();
110+
File f1 = new File("safe/file.txt");
111+
112+
// GOOD: ensure that the path is normalized and is then used as the
113+
// second argument to a `File` constructor
114+
Path normalizedFilename = Paths.get(filename).normalize().toAbsolutePath();
115+
String normalizedFilenameStr = normalizedFilename.toString();
116+
File f2 = new File(f1, normalizedFilenameStr);
117+
f2.exists();
118+
119+
// Only sanitize `f2`; `filename` and `normalizedFilenameStr` are still tainted
120+
BufferedReader fileReader = new BufferedReader(new FileReader(filename)); // $ hasTaintFlow
121+
BufferedReader fileReader2 = new BufferedReader(new FileReader(normalizedFilenameStr)); // $ hasTaintFlow
122+
}
89123
}

0 commit comments

Comments
 (0)