Temp Dir Info Disclosure: Final pass and add documentation

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

@@ -0,0 +1,47 @@
+<!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.Files#createTempDirectory</a></li>
+    <li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempFile">java.nio.file.Files#createTempFile</a></li>
+<ul>
+Otherwise, create the file/directory by manually specificfying the expected posix file permissions.
+Eg. <code>PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))</code>
+<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 allowing 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 protecting their contents.</p>
+
+<sample src="TempDirUsageSafe.java"/>
+
+<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>
+</references>
+</qhelp>

java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosureFromMethodCall.qhelp

@@ -0,0 +1,5 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<include src="TempDirLocalInformationDisclosure.qhelp" /></qhelp>

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

@@ -4,9 +4,10 @@
  * @kind problem
  * @problem.severity warning
  * @precision very-high
- * @id java/local-information-disclosure
+ * @id java/local-temp-file-or-directory-information-disclosure-method
  * @tags security
  *       external/cwe/cwe-200
+ *       external/cwe/cwe-732
  */
 
 import TempDirUtils
@@ -25,7 +26,7 @@ class TempDirSystemGetPropertyToAnyConfig extends TaintTracking::Configuration {
   TempDirSystemGetPropertyToAnyConfig() { this = "TempDirSystemGetPropertyToAnyConfig" }
 
   override predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDir
+    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
   }
 
   override predicate isSink(DataFlow::Node source) { any() }
@@ -45,6 +46,7 @@ class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCre
     this.getMethod() instanceof MethodFileCreateTempFile and
     (
       this.getNumArgument() = 2 or
+      // Vulnerablilty exists when the last argument is `null`
       getArgument(2) instanceof NullLiteral or
       // There exists a flow from the 'java.io.tmpdir' system property to this argument
       exists(TempDirSystemGetPropertyToAnyConfig config |

java/ql/src/Security/CWE/CWE-200/TempDirLocalInformationDisclosureFromSystemProperty.qhelp

@@ -0,0 +1,5 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<include src="TempDirLocalInformationDisclosure.qhelp" /></qhelp>

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

@@ -4,9 +4,10 @@
  * @kind path-problem
  * @problem.severity warning
  * @precision very-high
- * @id java/local-information-disclosure
+ * @id java/local-temp-file-or-directory-information-disclosure-path
  * @tags security
  *       external/cwe/cwe-200
+ *       external/cwe/cwe-732
  */
 
 import TempDirUtils
@@ -24,6 +25,9 @@ private class MethodFileSystemFileCreation extends Method {
 
 abstract private class FileCreationSink extends DataFlow::Node { }
 
+/**
+ * Sink for tainted `File` having a file or directory creation method called on it.
+ */
 private class FileFileCreationSink extends FileCreationSink {
   FileFileCreationSink() {
     exists(MethodAccess ma |
@@ -33,12 +37,19 @@ private class FileFileCreationSink extends FileCreationSink {
   }
 }
 
+/**
+ * Sink for if tained File/Path having some `Files` method called on it that creates a file or directory.
+ */
 private class FilesFileCreationSink extends FileCreationSink {
   FilesFileCreationSink() {
     exists(FilesVulnerableCreationMethodAccess ma | ma.getArgument(0) = this.asExpr())
   }
 }
 
+/**
+ * Captures all of the vulnerable methods on `Files` that create files/directories without explicitly
+ * setting the permissions.
+ */
 private class FilesVulnerableCreationMethodAccess extends MethodAccess {
   FilesVulnerableCreationMethodAccess() {
     getMethod().getDeclaringType().hasQualifiedName("java.nio.file", "Files") and
@@ -54,7 +65,7 @@ private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Conf
   TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" }
 
   override predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDir
+    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
   }
 
   override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {

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

@@ -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-------`
+    }
+}

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

@@ -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--`
+    }
+}

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

@@ -1,8 +1,32 @@
 import java
 import semmle.code.java.dataflow.FlowSources
 
-class MethodAccessSystemGetPropertyTempDir extends MethodAccessSystemGetProperty {
-  MethodAccessSystemGetPropertyTempDir() { this.hasCompileTimeConstantGetPropertyName("java.io.tmpdir") }
+/**
+ * A method that returns a `String` or `File` that has been tainted by `System.getProperty("java.io.tmpdir")`.
+ */
+abstract class MethodAccessSystemGetPropertyTempDirTainted extends MethodAccess { }
+
+/**
+ * Method access `System.getProperty("java.io.tmpdir")`.
+ */
+private class MethodAccessSystemGetPropertyTempDir extends MethodAccessSystemGetPropertyTempDirTainted,
+  MethodAccessSystemGetProperty {
+  MethodAccessSystemGetPropertyTempDir() {
+    this.hasCompileTimeConstantGetPropertyName("java.io.tmpdir")
+  }
+}
+
+/**
+ * A method call to the `org.apache.commons.io.FileUtils` methods `getTempDirectory` or `getTempDirectoryPath`.
+ */
+private class MethodAccessApacheFileUtilsTempDir extends MethodAccessSystemGetPropertyTempDirTainted {
+  MethodAccessApacheFileUtilsTempDir() {
+    exists(Method m |
+      m.getDeclaringType().hasQualifiedName("org.apache.commons.io", "FileUtils") and
+      m.hasName(["getTempDirectory", "getTempDirectoryPath"]) and
+      this.getMethod() = m
+    )
+  }
 }
 
 /**
@@ -20,7 +44,7 @@ private predicate isTaintedFileCreation(Expr expSource, Expr exprDest) {
 }
 
 /**
- * Any `File` methods that 
+ * Any `File` methods that
  */
 private class TaintFollowingFileMethod extends Method {
   TaintFollowingFileMethod() {