Java: CWE-200: Temp directory local information disclosure vulnerability

Author: JLLeitschuh

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

@@ -0,0 +1,72 @@
+/**
+ * @name Temporary Directory Local information disclosure
+ * @description Detect local information disclosure via the java temporary directory
+ * @kind problem
+ * @problem.severity warning
+ * @precision very-high
+ * @id java/local-information-disclosure
+ * @tags security
+ *       external/cwe/cwe-200
+ */
+
+import TempDirUtils
+
+/**
+ * All `java.io.File::createTempFile` methods.
+ */
+class MethodFileCreateTempFile extends Method {
+  MethodFileCreateTempFile() {
+    this.getDeclaringType() instanceof TypeFile and
+    this.hasName("createTempFile")
+  }
+}
+
+class TempDirSystemGetPropertyToAnyConfig extends TaintTracking::Configuration {
+  TempDirSystemGetPropertyToAnyConfig() { this = "TempDirSystemGetPropertyToAnyConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDir
+  }
+
+  override predicate isSink(DataFlow::Node source) { any() }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalFileTaintStep(node1, node2)
+  }
+}
+
+abstract class MethodAccessInsecureFileCreation extends MethodAccess { }
+
+/**
+ * Insecure calls to `java.io.File::createTempFile`.
+ */
+class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCreation {
+  MethodAccessInsecureFileCreateTempFile() {
+    this.getMethod() instanceof MethodFileCreateTempFile and
+    (
+      this.getNumArgument() = 2 or
+      getArgument(2) instanceof NullLiteral or
+      // There exists a flow from the 'java.io.tmpdir' system property to this argument
+      exists(TempDirSystemGetPropertyToAnyConfig config |
+        config.hasFlowTo(DataFlow::exprNode(getArgument(2)))
+      )
+    )
+  }
+}
+
+class MethodGuavaFilesCreateTempFile extends Method {
+  MethodGuavaFilesCreateTempFile() {
+    getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and
+    hasName("createTempDir")
+  }
+}
+
+class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation {
+  MethodAccessInsecureGuavaFilesCreateTempFile() {
+    getMethod() instanceof MethodGuavaFilesCreateTempFile
+  }
+}
+
+from MethodAccessInsecureFileCreation methodAccess
+select methodAccess,
+  "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

@@ -0,0 +1,49 @@
+/**
+ * @name Temporary Directory Local information disclosure
+ * @description Detect local information disclosure via the java temporary directory
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision very-high
+ * @id java/local-information-disclosure
+ * @tags security
+ *       external/cwe/cwe-200
+ */
+
+import TempDirUtils
+import DataFlow::PathGraph
+
+private class MethodFileSystemCreation extends Method {
+  MethodFileSystemCreation() {
+    getDeclaringType() instanceof TypeFile and
+    (
+      hasName("mkdir") or
+      hasName("createNewFile")
+    )
+  }
+}
+
+private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Configuration {
+  TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToAnyConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDir
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists (MethodAccess ma |
+      ma.getMethod() instanceof MethodFileSystemCreation and
+      ma.getQualifier() = sink.asExpr()
+    )
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalFileTaintStep(node1, node2)
+  }
+}
+
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, TempDirSystemGetPropertyToCreateConfig conf
+where conf.hasFlowPath(source, sink)
+select source.getNode(), source, sink,
+  "Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users.", source.getNode(),
+  "system temp directory"

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

@@ -0,0 +1,43 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+
+class MethodAccessSystemGetPropertyTempDir extends MethodAccessSystemGetProperty {
+  MethodAccessSystemGetPropertyTempDir() { this.hasPropertyName("java.io.tmpdir") }
+}
+
+/**
+ * Find dataflow from the temp directory system property to the `File` constructor.
+ * Examples:
+ *  - `new File(System.getProperty("java.io.tmpdir"))`
+ *  - `new File(new File(System.getProperty("java.io.tmpdir")), "/child")`
+ */
+private predicate isTaintedFileCreation(Expr expSource, Expr exprDest) {
+  exists(ConstructorCall construtorCall |
+    construtorCall.getConstructedType() instanceof TypeFile and
+    construtorCall.getArgument(0) = expSource and
+    construtorCall = exprDest
+  )
+}
+
+private class TaintFollowingFileMethod extends Method {
+  TaintFollowingFileMethod() {
+    getDeclaringType() instanceof TypeFile and
+    (
+      hasName("getAbsoluteFile") or
+      hasName("getCanonicalFile")
+    )
+  }
+}
+
+private predicate isTaintFollowingFileTransformation(Expr expSource, Expr exprDest) {
+  exists(MethodAccess fileMethodAccess |
+    fileMethodAccess.getMethod() instanceof TaintFollowingFileMethod and
+    fileMethodAccess.getQualifier() = expSource and
+    fileMethodAccess = exprDest
+  )
+}
+
+predicate isAdditionalFileTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+  isTaintedFileCreation(node1.asExpr(), node2.asExpr()) or
+  isTaintFollowingFileTransformation(node1.asExpr(), node2.asExpr())
+}

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

@@ -211,6 +211,16 @@ class MethodSystemGetProperty extends Method {
   }
 }
 
+class MethodAccessSystemGetProperty extends MethodAccess {
+  MethodAccessSystemGetProperty() {
+    getMethod() instanceof MethodSystemGetProperty
+  }
+
+  predicate hasPropertyName(string propertyName) {
+    this.getArgument(0).(CompileTimeConstantExpr).getStringValue() = propertyName
+  }
+}
+
 /**
  * Any method named `exit` on class `java.lang.Runtime` or `java.lang.System`.
  */

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

@@ -307,7 +307,7 @@ private predicate formatStringValue(Expr e, string fmtvalue) {
     or
     exists(Field f |
       e = f.getAnAccess() and
-      f.getDeclaringType().hasQualifiedName("java.io", "File") and
+      f.getDeclaringType() instanceof TypeFile and
       fmtvalue = "x" // dummy value
     |
       f.hasName("pathSeparator") or

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

@@ -244,7 +244,7 @@ class SecurityManagerClass extends Class {
 /** A class involving file input or output. */
 class FileInputOutputClass extends Class {
   FileInputOutputClass() {
-    this.hasQualifiedName("java.io", "File") or
+    this instanceof TypeFile or
     this.hasQualifiedName("java.io", "FileDescriptor") or
     this.hasQualifiedName("java.io", "FileInputStream") or
     this.hasQualifiedName("java.io", "FileOutputStream") or

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

@@ -67,7 +67,7 @@ private VarAccess getFileForPathConversion(Expr pathExpr) {
       fileToPath = pathExpr and
       result = fileToPath.getQualifier() and
       fileToPath.getMethod().hasName("toPath") and
-      fileToPath.getMethod().getDeclaringType().hasQualifiedName("java.io", "File")
+      fileToPath.getMethod().getDeclaringType() instanceof TypeFile
     )
     or
     // 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

@@ -0,0 +1,22 @@
+package com.google.common.io;
+
+import java.io.File;
+
+public class Files {
+    /** Maximum loop count when creating temp directories. */
+    private static final int TEMP_DIR_ATTEMPTS = 10000;
+
+    public static File createTempDir() {
+        File baseDir = new File(System.getProperty("java.io.tmpdir"));
+        String baseName = System.currentTimeMillis() + "-";
+
+        for (int counter = 0; counter < TEMP_DIR_ATTEMPTS; counter++) {
+            File tempDir = new File(baseDir, baseName + counter);
+            if (tempDir.mkdir()) {
+                return tempDir;
+            }
+        }
+        throw new IllegalStateException("Failed to create directory within " + TEMP_DIR_ATTEMPTS + " attempts (tried "
+                + baseName + "0 to " + baseName + (TEMP_DIR_ATTEMPTS - 1) + ')');
+    }
+}

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

@@ -0,0 +1 @@
+Security/CWE/CWE-200/TempDirLocalInformationDisclosure1.ql

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

@@ -0,0 +1 @@
+Security/CWE/CWE-200/TempDirLocalInformationDisclosure2.ql