github · smowton · Feb 14, 2022 · Oct 2, 2020 · Oct 14, 2020 · Dec 8, 2020
@@ -325,7 +325,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

@@ -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

@@ -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`.

@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Local information disclosure can occur when files/directories are written into
+directories that are shared between all users on the system.</p>
+
+<p>On most <a href="https://en.wikipedia.org/wiki/Unix-like">unix-like</a> systems,
+the system temporary directory is shared between local users.
+If files/directories are created within the system temporary directory without using
+APIs that explicitly set the correct file permissions, local information disclosure
+can occur.</p>
+
+<p>Depending upon the particular file contents exposed, this vulnerability can have a
+<a href="https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:L/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N&amp;version=3.1">CVSSv3.1 base score of 6.2/10</a>.</p>
+</overview>
+
+<recommendation>
+<p>Use JDK methods that specifically protect against this vulnerability:</p>
+<ul>
+    <li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempDirectory-java.nio.file.Path-java.lang.String-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files.createTempDirectory</a></li>
+    <li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempFile-java.nio.file.Path-java.lang.String-java.lang.String-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files.createTempFile</a></li>
+</ul>
+<p>Otherwise, create the file/directory by manually specifying the expected posix file permissions.
+For example: <code>PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))</code></p>
+<ul>
+    <li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createFile-java.nio.file.Path-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files.createFile</a></li>
+    <li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createDirectory-java.nio.file.Path-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files.createDirectory</a></li>
+    <li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createDirectories-java.nio.file.Path-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files.createDirectories</a></li>
+</ul>
+</recommendation>
+
+<example>
+<p>In the following example, files and directories are created with file permissions that allow other local users to read their contents.</p>
+
+<sample src="TempDirUsageVulnerable.java"/>
+
+<p>In the following example, files and directories are created with file permissions that protect their contents.</p>
+
+<sample src="TempDirUsageSafe.java"/>
+</example>
+
+<references>
+    <li>OSWAP: <a href="https://owasp.org/www-community/vulnerabilities/Insecure_Temporary_File">Insecure Temporary File</a>.</li>
+    <li>CERT: <a href="https://wiki.sei.cmu.edu/confluence/display/java/FIO00-J.+Do+not+operate+on+files+in+shared+directories">FIO00-J. Do not operate on files in shared directories</a></li>
+</references>
+</qhelp>
@@ -0,0 +1,211 @@
+/**
+ * @name Temporary Directory Local information disclosure
- * @name Temporary Directory Local information disclosure
+ * @name Local information disclosure in a temporary directory
- * @name Temporary Directory Local information disclosure
+ * @name Local information disclosure in a temporary directory
+ * @description Writing information without explicit permissions to a shared temporary directory may disclose it to other users.
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision very-high
+ * @id java/local-temp-file-or-directory-information-disclosure
+ * @tags security
+ *       external/cwe/cwe-200
+ *       external/cwe/cwe-732
+ */
+
+import java
+import TempDirUtils
+import DataFlow::PathGraph
+
+private class MethodFileSystemFileCreation extends Method {
+  MethodFileSystemFileCreation() {
+    this.getDeclaringType() instanceof TypeFile and
+    this.hasName(["mkdir", "mkdirs", "createNewFile"])
+  }
+}
+
+abstract private class FileCreationSink extends DataFlow::Node { }
+
+/**
+ * The qualifier of a call to one of `File`'s file-creating or directory-creating methods,
+ * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`.
+ */
+private class FileFileCreationSink extends FileCreationSink {
+  FileFileCreationSink() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof MethodFileSystemFileCreation and
+      ma.getQualifier() = this.asExpr()
+    )
+  }
+}
+
+/**
+ * The argument to a call to one of `Files` file-creating or directory-creating methods,
+ * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`.
+ */
+private class FilesFileCreationSink extends FileCreationSink {
+  FilesFileCreationSink() {
+    exists(FilesVulnerableCreationMethodAccess ma | ma.getArgument(0) = this.asExpr())
+  }
+}
+
+/**
+ * A call to a `Files` method that create files/directories without explicitly
+ * setting the newly-created file or directory's permissions.
+ */
+private class FilesVulnerableCreationMethodAccess extends MethodAccess {
+  FilesVulnerableCreationMethodAccess() {
+    exists(Method m |
+      m = this.getMethod() and
+      m.getDeclaringType().hasQualifiedName("java.nio.file", "Files")
+    |
+      m.hasName(["write", "newBufferedWriter", "newOutputStream"])
+      or
+      m.hasName(["createFile", "createDirectory", "createDirectories"]) and
+      this.getNumArgument() = 1
+      or
+      m.hasName("newByteChannel") and
+      this.getNumArgument() = 2
+    )
+  }
+}
+
+/**
+ * A call to a `File` method that create files/directories with a specific set of permissions explicitly set.
+ * We can safely assume that any calls to these methods with explicit `PosixFilePermissions.asFileAttribute`
+ * contains a certain level of intentionality behind it.
+ */
+private class FilesSanitizingCreationMethodAccess extends MethodAccess {
+  FilesSanitizingCreationMethodAccess() {
+    exists(Method m |
+      m = this.getMethod() and
+      m.getDeclaringType().hasQualifiedName("java.nio.file", "Files")
+    |
+      m.hasName(["createFile", "createDirectory", "createDirectories"]) and
+      this.getNumArgument() = 2
+    )
+  }
+}
+
+/**
+ * The temp directory argument to a call to `java.io.File::createTempFile`,
+ * treated as a sink by `TempDirSystemGetPropertyToCreateConfig`.
+ */
+private class FileCreateTempFileSink extends FileCreationSink {
+  FileCreateTempFileSink() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof MethodFileCreateTempFile and ma.getArgument(2) = this.asExpr()
+    )
+  }
+}
+
+private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Configuration {
+  TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
+  }
+
+  /**
+   * 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")`
+   */
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    isAdditionalFileTaintStep(node1, node2)
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof FileCreationSink }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    exists(FilesSanitizingCreationMethodAccess sanitisingMethodAccess |
+      sanitizer.asExpr() = sanitisingMethodAccess.getArgument(0)
+    )
+  }
+}
+
+//
+// Begin configuration for tracking single-method calls that are vulnerable.
+//
+
+/**
+ * A `MethodAccess` against a method that creates a temporary file or directory in a shared temporary directory.
+ */
+abstract class MethodAccessInsecureFileCreation extends MethodAccess {
+  /**
+   * Gets the type of entity created (e.g. `file`, `directory`, ...).
+   */
+  abstract string getFileSystemEntityType();
+}
+
+/**
+ * An insecure call to `java.io.File.createTempFile`.
+ */
+class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCreation {
+  MethodAccessInsecureFileCreateTempFile() {
+    this.getMethod() instanceof MethodFileCreateTempFile and
+    (
+      // `File.createTempFile(string, string)` always uses the default temporary directory
+      this.getNumArgument() = 2
+      or
+      // The default temporary directory is used when the last argument of `File.createTempFile(string, string, File)` is `null`
+      DataFlow::localExprFlow(any(NullLiteral n), getArgument(2))
+    )
+  }
+
+  override string getFileSystemEntityType() { result = "file" }
+}
+
+/**
+ * The `com.google.common.io.Files.createTempDir` method.
+ */
+class MethodGuavaFilesCreateTempFile extends Method {
+  MethodGuavaFilesCreateTempFile() {
+    getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and
+    hasName("createTempDir")
+  }
+}
+
+/**
+ * A call to the `com.google.common.io.Files.createTempDir` method.
+ */
+class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation {
+  MethodAccessInsecureGuavaFilesCreateTempFile() {
+    getMethod() instanceof MethodGuavaFilesCreateTempFile
+  }
+
+  override string getFileSystemEntityType() { result = "directory" }
+}
+
+/**
+ * This is a hack: we include use of inherently insecure methods, which don't have any associated
+ * flow path, in with results describing a path from reading `java.io.tmpdir` or similar to use
+ * in a file creation op.
+ *
+ * We achieve this by making inherently-insecure method invocations both a source and a sink in
+ * this configuration, resulting in a zero-length path which is type-compatible with the actual
+ * path-flow results.
+ */
+class InsecureMethodPseudoConfiguration extends DataFlow::Configuration {
+  InsecureMethodPseudoConfiguration() { this = "InsecureMethodPseudoConfiguration" }
+
+  override predicate isSource(DataFlow::Node node) {
+    node.asExpr() instanceof MethodAccessInsecureFileCreation
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    node.asExpr() instanceof MethodAccessInsecureFileCreation
+  }
+}
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, string message
+where
+  any(TempDirSystemGetPropertyToCreateConfig conf).hasFlowPath(source, sink) and
+  message =
+    "Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users."
+  or
+  any(InsecureMethodPseudoConfiguration conf).hasFlowPath(source, sink) and
+  // Note this message has no "$@" placeholder, so the "system temp directory" template parameter below is not used.
+  message =
+    "Local information disclosure vulnerability due to use of " +
+      source.getNode().asExpr().(MethodAccessInsecureFileCreation).getFileSystemEntityType() +
+      " readable by other local users."
+select source.getNode(), source, sink, message, source.getNode(), "system temp directory"
@@ -0,0 +1,19 @@
+import java.nio.file.Files;
+import java.nio.file.attribute.PosixFilePermission;
+import java.nio.file.attribute.PosixFilePermissions;
+import java.util.EnumSet;
+
+public class TempDirUsageSafe {
+    void exampleSafe() throws IOException {
+        Path temp1 = Files.createTempFile("random", ".txt"); // GOOD: File has permissions `-rw-------`
+
+        Path temp2 = Files.createTempDirectory("random-directory"); // GOOD: File has permissions `drwx------`
+
+        File tempDirChildFile = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt");
+        Files.createFile(
+            tempDirChildFile.toPath(),
+            tempDirChild.toPath(),
+            PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))
+        ); // GOOD: Good has permissions `-rw-------`
+    }
+}
@@ -0,0 +1,19 @@
+import java.io.File;
+
+public class TempDirUsageVulnerable {
+    void exampleVulnerable() {
+        File temp1 = File.createTempFile("random", ".txt"); // BAD: File has permissions `-rw-r--r--`
+
+        File temp2 = File.createTempFile("random", "file", null); // BAD: File has permissions `-rw-r--r--`
+
+        File systemTempDir = new File(System.getProperty("java.io.tmpdir"));
+        File temp3 = File.createTempFile("random", "file", systemTempDir); // BAD: File has permissions `-rw-r--r--`
+
+        File tempDir = com.google.common.io.Files.createTempDir(); // BAD: CVE-2020-8908: Directory has permissions `drwxr-xr-x`
+
+        new File(System.getProperty("java.io.tmpdir"), "/child").mkdir(); // BAD: Directory has permissions `-rw-r--r--`
+
+        File tempDirChildFile = new File(System.getProperty("java.io.tmpdir"), "/child-create-file.txt");
+        Files.createFile(tempDirChildFile.toPath()); // BAD: File has permissions `-rw-r--r--`
+    }
+}