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

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

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

java/ql/lib/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/lib/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/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosure.qhelp

@@ -0,0 +1,49 @@
+<!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>

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

@@ -0,0 +1,258 @@
+/**
+ * @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 medium
+ * @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
+import semmle.code.java.dataflow.TaintTracking2
+
+abstract private class MethodFileSystemFileCreation extends Method {
+  MethodFileSystemFileCreation() { this.getDeclaringType() instanceof TypeFile }
+}
+
+private class MethodFileDirectoryCreation extends MethodFileSystemFileCreation {
+  MethodFileDirectoryCreation() { this.hasName(["mkdir", "mkdirs"]) }
+}
+
+private class MethodFileFileCreation extends MethodFileSystemFileCreation {
+  MethodFileFileCreation() { this.hasName("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 and
+    not any(TempDirSystemGetPropertyDirectlyToMkdirConfig config).hasFlowTo(sink)
+  }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    exists(FilesSanitizingCreationMethodAccess sanitisingMethodAccess |
+      sanitizer.asExpr() = sanitisingMethodAccess.getArgument(0)
+    )
+  }
+}
+
+/**
+ * Configuration that tracks calls to to `mkdir` or `mkdirs` that are are directly on the temp directory system property.
+ * Examples:
+ * - `File tempDir = new File(System.getProperty("java.io.tmpdir")); tempDir.mkdir();`
+ * - `File tempDir = new File(System.getProperty("java.io.tmpdir")); tempDir.mkdirs();`
+ *
+ * These are examples of code that is simply verifying that the temp directory exists.
+ * As such, this code pattern is filtered out as an explicit vulnerability in
+ * `TempDirSystemGetPropertyToCreateConfig::isSink`.
+ */
+private class TempDirSystemGetPropertyDirectlyToMkdirConfig extends TaintTracking2::Configuration {
+  TempDirSystemGetPropertyDirectlyToMkdirConfig() {
+    this = "TempDirSystemGetPropertyDirectlyToMkdirConfig"
+  }
+
+  override predicate isSource(DataFlow::Node node) {
+    exists(
+      MethodAccessSystemGetPropertyTempDirTainted propertyGetMethodAccess, DataFlow::Node callSite
+    |
+      DataFlow::localFlow(DataFlow::exprNode(propertyGetMethodAccess), callSite)
+    |
+      isFileConstructorArgument(callSite.asExpr(), node.asExpr(), 1)
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node) {
+    exists(MethodAccess ma | ma.getMethod() instanceof MethodFileDirectoryCreation |
+      ma.getQualifier() = node.asExpr()
+    )
+  }
+
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    isFileConstructorArgument(sanitizer.asExpr(), _, _)
+  }
+}
+
+//
+// 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), this.getArgument(2))
+    )
+  }
+
+  override string getFileSystemEntityType() { result = "file" }
+}
+
+/**
+ * The `com.google.common.io.Files.createTempDir` method.
+ */
+class MethodGuavaFilesCreateTempFile extends Method {
+  MethodGuavaFilesCreateTempFile() {
+    this.getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and
+    this.hasName("createTempDir")
+  }
+}
+
+/**
+ * A call to the `com.google.common.io.Files.createTempDir` method.
+ */
+class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation {
+  MethodAccessInsecureGuavaFilesCreateTempFile() {
+    this.getMethod() instanceof MethodGuavaFilesCreateTempFile
+  }
+
+  override string getFileSystemEntityType() { result = "directory" }
+}
+
+/**
+ * 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."
+  ) and
+  not isPermissionsProtectedTempDirUse(sink.getNode())
+select source.getNode(), source, sink, message, source.getNode(), "system temp directory"