Skip to content

Commit d53d77c

Browse files
JLLeitschuhJLLeitschuh
committed
Java: CWE-200: Temp directory local information disclosure vulnerability
1 parent 768e519 commit d53d77c

File tree

11 files changed

+260
-3
lines changed

11 files changed

+260
-3
lines changed

‎java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure1.ql

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* @name Temporary Directory Local information disclosure
3+
* @description Detect local information disclosure via the java temporary directory
4+
* @kind problem
5+
* @problem.severity warning
6+
* @precision very-high
7+
* @id java/local-information-disclosure
8+
* @tags security
9+
* external/cwe/cwe-200
10+
*/
11+
12+
import TempDirUtils
13+
14+
/**
15+
* All `java.io.File::createTempFile` methods.
16+
*/
17+
class MethodFileCreateTempFile extends Method {
18+
MethodFileCreateTempFile() {
19+
this.getDeclaringType() instanceof TypeFile and
20+
this.hasName("createTempFile")
21+
}
22+
}
23+
24+
class TempDirSystemGetPropertyToAnyConfig extends TaintTracking::Configuration {
25+
TempDirSystemGetPropertyToAnyConfig() { this = "TempDirSystemGetPropertyToAnyConfig" }
26+
27+
override predicate isSource(DataFlow::Node source) {
28+
source.asExpr() instanceof MethodAccessSystemGetPropertyTempDir
29+
}
30+
31+
override predicate isSink(DataFlow::Node source) { any() }
32+
33+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
34+
isAdditionalFileTaintStep(node1, node2)
35+
}
36+
}
37+
38+
abstract class MethodAccessInsecureFileCreation extends MethodAccess { }
39+
40+
/**
41+
* Insecure calls to `java.io.File::createTempFile`.
42+
*/
43+
class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCreation {
44+
MethodAccessInsecureFileCreateTempFile() {
45+
this.getMethod() instanceof MethodFileCreateTempFile and
46+
(
47+
this.getNumArgument() = 2 or
48+
getArgument(2) instanceof NullLiteral or
49+
// There exists a flow from the 'java.io.tmpdir' system property to this argument
50+
exists(TempDirSystemGetPropertyToAnyConfig config |
51+
config.hasFlowTo(DataFlow::exprNode(getArgument(2)))
52+
)
53+
)
54+
}
55+
}
56+
57+
class MethodGuavaFilesCreateTempFile extends Method {
58+
MethodGuavaFilesCreateTempFile() {
59+
getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and
60+
hasName("createTempDir")
61+
}
62+
}
63+
64+
class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation {
65+
MethodAccessInsecureGuavaFilesCreateTempFile() {
66+
getMethod() instanceof MethodGuavaFilesCreateTempFile
67+
}
68+
}
69+
70+
from MethodAccessInsecureFileCreation methodAccess
71+
select methodAccess,
72+
"Local information disclosure vulnerability due to use of file or directory readable by other local users."

‎java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure2.ql

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @name Temporary Directory Local information disclosure
3+
* @description Detect local information disclosure via the java temporary directory
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @precision very-high
7+
* @id java/local-information-disclosure
8+
* @tags security
9+
* external/cwe/cwe-200
10+
*/
11+
12+
import TempDirUtils
13+
import DataFlow::PathGraph
14+
15+
private class MethodFileSystemCreation extends Method {
16+
MethodFileSystemCreation() {
17+
getDeclaringType() instanceof TypeFile and
18+
(
19+
hasName("mkdir") or
20+
hasName("createNewFile")
21+
)
22+
}
23+
}
24+
25+
private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Configuration {
26+
TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" }
27+
28+
override predicate isSource(DataFlow::Node source) {
29+
source.asExpr() instanceof MethodAccessSystemGetPropertyTempDir
30+
}
31+
32+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
33+
isAdditionalFileTaintStep(node1, node2)
34+
}
35+
36+
override predicate isSink(DataFlow::Node sink) {
37+
exists (MethodAccess ma |
38+
ma.getMethod() instanceof MethodFileSystemCreation and
39+
ma.getQualifier() = sink.asExpr()
40+
)
41+
}
42+
}
43+
44+
from DataFlow::PathNode source, DataFlow::PathNode sink, TempDirSystemGetPropertyToCreateConfig conf
45+
where conf.hasFlowPath(source, sink)
46+
select source.getNode(), source, sink,
47+
"Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users.", source.getNode(),
48+
"system temp directory"

‎java/ql/src/Security/CWE/CWE-200/TempDirUtils.qll

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import java
2+
import semmle.code.java.dataflow.FlowSources
3+
4+
class MethodAccessSystemGetPropertyTempDir extends MethodAccessSystemGetProperty {
5+
MethodAccessSystemGetPropertyTempDir() { this.hasCompileTimeConstantGetPropertyName("java.io.tmpdir") }
6+
}
7+
8+
/**
9+
* Find dataflow from the temp directory system property to the `File` constructor.
10+
* Examples:
11+
* - `new File(System.getProperty("java.io.tmpdir"))`
12+
* - `new File(new File(System.getProperty("java.io.tmpdir")), "/child")`
13+
*/
14+
private predicate isTaintedFileCreation(Expr expSource, Expr exprDest) {
15+
exists(ConstructorCall construtorCall |
16+
construtorCall.getConstructedType() instanceof TypeFile and
17+
construtorCall.getArgument(0) = expSource and
18+
construtorCall = exprDest
19+
)
20+
}
21+
22+
/**
23+
* Any `File` methods that
24+
*/
25+
private class TaintFollowingFileMethod extends Method {
26+
TaintFollowingFileMethod() {
27+
getDeclaringType() instanceof TypeFile and
28+
(
29+
hasName("getAbsoluteFile") or
30+
hasName("getCanonicalFile")
31+
)
32+
}
33+
}
34+
35+
private predicate isTaintFollowingFileTransformation(Expr expSource, Expr exprDest) {
36+
exists(MethodAccess fileMethodAccess |
37+
fileMethodAccess.getMethod() instanceof TaintFollowingFileMethod and
38+
fileMethodAccess.getQualifier() = expSource and
39+
fileMethodAccess = exprDest
40+
)
41+
}
42+
43+
predicate isAdditionalFileTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
44+
isTaintedFileCreation(node1.asExpr(), node2.asExpr()) or
45+
isTaintFollowingFileTransformation(node1.asExpr(), node2.asExpr())
46+
}

‎java/ql/src/semmle/code/java/JDK.qll

+17
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,23 @@ class MethodSystemGetProperty extends Method {
211211
}
212212
}
213213

214+
/**
215+
* Any method access to a method named `getProperty` on class `java.lang.System`.
216+
*/
217+
class MethodAccessSystemGetProperty extends MethodAccess {
218+
MethodAccessSystemGetProperty() {
219+
getMethod() instanceof MethodSystemGetProperty
220+
}
221+
222+
/**
223+
* Holds true if this is a compile-time constant call for the specified `propertyName`.
224+
* Eg. `System.getProperty("user.dir")`.
225+
*/
226+
predicate hasCompileTimeConstantGetPropertyName(string propertyName) {
227+
this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName
228+
}
229+
}
230+
214231
/**
215232
* Any method named `exit` on class `java.lang.Runtime` or `java.lang.System`.
216233
*/

‎java/ql/src/semmle/code/java/StringFormat.qll

+1-1
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ private predicate formatStringValue(Expr e, string fmtvalue) {
307307
or
308308
exists(Field f |
309309
e = f.getAnAccess() and
310-
f.getDeclaringType().hasQualifiedName("java.io", "File") and
310+
f.getDeclaringType() instanceof TypeFile and
311311
fmtvalue = "x" // dummy value
312312
|
313313
f.hasName("pathSeparator") or

‎java/ql/src/semmle/code/java/frameworks/javaee/ejb/EJBRestrictions.qll

+1-1
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ class SecurityManagerClass extends Class {
244244
/** A class involving file input or output. */
245245
class FileInputOutputClass extends Class {
246246
FileInputOutputClass() {
247-
this.hasQualifiedName("java.io", "File") or
247+
this instanceof TypeFile or
248248
this.hasQualifiedName("java.io", "FileDescriptor") or
249249
this.hasQualifiedName("java.io", "FileInputStream") or
250250
this.hasQualifiedName("java.io", "FileOutputStream") or

‎java/ql/src/semmle/code/java/security/FileWritable.qll

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private VarAccess getFileForPathConversion(Expr pathExpr) {
6767
fileToPath = pathExpr and
6868
result = fileToPath.getQualifier() and
6969
fileToPath.getMethod().hasName("toPath") and
70-
fileToPath.getMethod().getDeclaringType().hasQualifiedName("java.io", "File")
70+
fileToPath.getMethod().getDeclaringType() instanceof TypeFile
7171
)
7272
or
7373
// Look for the pattern `Paths.get(file.get*Path())` for converting between a `File` and a `Path`.

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/Files.java

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package com.google.common.io;
2+
3+
import java.io.File;
4+
5+
public class Files {
6+
/** Maximum loop count when creating temp directories. */
7+
private static final int TEMP_DIR_ATTEMPTS = 10000;
8+
9+
public static File createTempDir() {
10+
File baseDir = new File(System.getProperty("java.io.tmpdir"));
11+
String baseName = System.currentTimeMillis() + "-";
12+
13+
for (int counter = 0; counter < TEMP_DIR_ATTEMPTS; counter++) {
14+
File tempDir = new File(baseDir, baseName + counter);
15+
if (tempDir.mkdir()) {
16+
return tempDir;
17+
}
18+
}
19+
throw new IllegalStateException("Failed to create directory within " + TEMP_DIR_ATTEMPTS + " attempts (tried "
20+
+ baseName + "0 to " + baseName + (TEMP_DIR_ATTEMPTS - 1) + ')');
21+
}
22+
}

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure1.qlref

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-200/TempDirLocalInformationDisclosure1.ql

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosure2.qlref

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-200/TempDirLocalInformationDisclosure2.ql

‎java/ql/test/query-tests/security/CWE-200/semmle/tests/Test.java

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
2+
import java.io.File;
3+
import com.google.common.io.Files;
4+
5+
public class Test {
6+
7+
void vulnerableFileCreateTempFile() {
8+
File temp = File.createTempFile("random", "file");
9+
}
10+
11+
void vulnerableFileCreateTempFileNull() {
12+
File temp = File.createTempFile("random", "file", null);
13+
}
14+
15+
void vulnerableFileCreateTempFileTainted() {
16+
File tempDir = new File(System.getProperty("java.io.tmpdir"));
17+
File temp = File.createTempFile("random", "file", tempDir);
18+
}
19+
20+
void vulnerableFileCreateTempFileChildTainted() {
21+
File tempDirChild = new File(new File(System.getProperty("java.io.tmpdir")), "/child");
22+
File temp = File.createTempFile("random", "file", tempDirChild);
23+
}
24+
25+
void vulnerableFileCreateTempFileCanonical() {
26+
File tempDir = new File(System.getProperty("java.io.tmpdir")).getCanonicalFile();
27+
File temp = File.createTempFile("random", "file", tempDir);
28+
}
29+
30+
void vulnerableFileCreateTempFileAbsolute() {
31+
File tempDir = new File(System.getProperty("java.io.tmpdir")).getAbsoluteFile();
32+
File temp = File.createTempFile("random", "file", tempDir);
33+
}
34+
35+
void safeFileCreateTempFileTainted() {
36+
/* Creating a temporary directoy in the current user directory is not a vulnerability. */
37+
File currentDirectory = new File(System.getProperty("user.dir"));
38+
File temp = File.createTempFile("random", "file", currentDirectory);
39+
}
40+
41+
void vulnerableGuavaFilesCreateTempDir() {
42+
File tempDir = Files.createTempDir();
43+
}
44+
45+
void vulnerableFileCreateTempFileMkdirTainted() {
46+
File tempDirChild = new File(System.getProperty("java.io.tmpdir"), "/child");
47+
tempDirChild.mkdir();
48+
}
49+
50+
}

0 commit comments

Comments
 (0)