Remove path flow tracking in 'TempDirLocalInformationDisclosureFromMethodCall'

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

@@ -12,32 +12,13 @@
 
 import TempDirUtils
 
-/**
- * All `java.io.File::createTempFile` methods.
+abstract class MethodAccessInsecureFileCreation extends MethodAccess {
+  /**
+   * Docstring describing the file system type (ie. file, directory, ect...) returned.
    */
-class MethodFileCreateTempFile extends Method {
-  MethodFileCreateTempFile() {
-    this.getDeclaringType() instanceof TypeFile and
-    this.hasName("createTempFile")
-  }
+  abstract string getFileSystemType();
 }
 
-class TempDirSystemGetPropertyToAnyConfig extends TaintTracking::Configuration {
-  TempDirSystemGetPropertyToAnyConfig() { this = "TempDirSystemGetPropertyToAnyConfig" }
-
-  override predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
-  }
-
-  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`.
  */
@@ -45,15 +26,14 @@ class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCre
   MethodAccessInsecureFileCreateTempFile() {
     this.getMethod() instanceof MethodFileCreateTempFile and
     (
-      this.getNumArgument() = 2 or
+      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 |
-        config.hasFlowTo(DataFlow::exprNode(getArgument(2)))
-      )
+      getArgument(2) instanceof NullLiteral
     )
   }
+
+  override string getFileSystemType() { result = "file" }
 }
 
 class MethodGuavaFilesCreateTempFile extends Method {
@@ -67,8 +47,11 @@ class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureF
   MethodAccessInsecureGuavaFilesCreateTempFile() {
     getMethod() instanceof MethodGuavaFilesCreateTempFile
   }
+
+  override string getFileSystemType() { result = "directory" }
 }
 
 from MethodAccessInsecureFileCreation methodAccess
 select methodAccess,
-  "Local information disclosure vulnerability due to use of file or directory readable by other local users."
+  "Local information disclosure vulnerability due to use of " + methodAccess.getFileSystemType() +
+    " readable by other local users."

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

@@ -56,7 +56,19 @@ private class FilesVulnerableCreationMethodAccess extends MethodAccess {
     (
       getMethod().hasName(["write", "newBufferedWriter", "newOutputStream"])
       or
-      getMethod().hasName(["createFile", "createDirectory", "createDirectories"]) and getNumArgument() = 1
+      getMethod().hasName(["createFile", "createDirectory", "createDirectories"]) and
+      getNumArgument() = 1
+    )
+  }
+}
+
+/**
+ * A call to `java.io.File::createTempFile` where the the system temp dir sinks to the last argument.
+ */
+private class FileCreateTempFileSink extends FileCreationSink {
+  FileCreateTempFileSink() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof MethodFileCreateTempFile and ma.getArgument(2) = this.asExpr()
     )
   }
 }

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

@@ -29,6 +29,16 @@ private class MethodAccessApacheFileUtilsTempDir extends MethodAccessSystemGetPr
   }
 }
 
+/**
+ * All `java.io.File::createTempFile` methods.
+ */
+class MethodFileCreateTempFile extends Method {
+  MethodFileCreateTempFile() {
+    this.getDeclaringType() instanceof TypeFile and
+    this.hasName("createTempFile")
+  }
+}
+
 /**
  * Find dataflow from the temp directory system property to the `File` constructor.
  * Examples:
@@ -44,7 +54,7 @@ private predicate isTaintedFileCreation(Expr expSource, Expr exprDest) {
 }
 
 /**
- * Any `File` methods that
+ * Any `File` methods where the temporary directory is still part of the root path.
  */
 private class TaintFollowingFileMethod extends Method {
   TaintFollowingFileMethod() {

java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosureFromMethodCall.expected

@@ -1,7 +1,3 @@
-| Test.java:15:21:15:57 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
-| Test.java:19:21:19:63 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
-| Test.java:24:21:24:66 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
-| Test.java:29:21:29:71 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
-| Test.java:34:21:34:66 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
-| Test.java:39:21:39:66 | createTempFile(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
-| Test.java:49:24:49:65 | createTempDir(...) | Local information disclosure vulnerability due to use of file or directory readable by other local users. |
+| Test.java:15:21:15:57 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. |
+| Test.java:19:21:19:63 | createTempFile(...) | Local information disclosure vulnerability due to use of file readable by other local users. |
+| Test.java:49:24:49:65 | createTempDir(...) | Local information disclosure vulnerability due to use of directory readable by other local users. |

java/ql/test/query-tests/security/CWE-200/semmle/tests/TempDirLocalInformationDisclosureFromSystemProperty.expected

@@ -1,5 +1,9 @@
 edges
 | Files.java:10:33:10:68 | getProperty(...) : String | Files.java:15:17:15:23 | tempDir |
+| Test.java:23:33:23:68 | getProperty(...) : String | Test.java:24:59:24:65 | tempDir |
+| Test.java:28:47:28:82 | getProperty(...) : String | Test.java:29:59:29:70 | tempDirChild |
+| Test.java:33:33:33:68 | getProperty(...) : String | Test.java:34:59:34:65 | tempDir |
+| Test.java:38:33:38:68 | getProperty(...) : String | Test.java:39:59:39:65 | tempDir |
 | Test.java:53:38:53:73 | getProperty(...) : String | Test.java:54:9:54:20 | tempDirChild |
 | Test.java:58:38:58:73 | getProperty(...) : String | Test.java:59:9:59:20 | tempDirChild |
 | Test.java:63:38:63:73 | getProperty(...) : String | Test.java:64:21:64:41 | toPath(...) |
@@ -12,6 +16,14 @@ edges
 nodes
 | Files.java:10:33:10:68 | getProperty(...) : String | semmle.label | getProperty(...) : String |
 | Files.java:15:17:15:23 | tempDir | semmle.label | tempDir |
+| Test.java:23:33:23:68 | getProperty(...) : String | semmle.label | getProperty(...) : String |
+| Test.java:24:59:24:65 | tempDir | semmle.label | tempDir |
+| Test.java:28:47:28:82 | getProperty(...) : String | semmle.label | getProperty(...) : String |
+| Test.java:29:59:29:70 | tempDirChild | semmle.label | tempDirChild |
+| Test.java:33:33:33:68 | getProperty(...) : String | semmle.label | getProperty(...) : String |
+| Test.java:34:59:34:65 | tempDir | semmle.label | tempDir |
+| Test.java:38:33:38:68 | getProperty(...) : String | semmle.label | getProperty(...) : String |
+| Test.java:39:59:39:65 | tempDir | semmle.label | tempDir |
 | Test.java:53:38:53:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
 | Test.java:54:9:54:20 | tempDirChild | semmle.label | tempDirChild |
 | Test.java:58:38:58:73 | getProperty(...) : String | semmle.label | getProperty(...) : String |
@@ -32,6 +44,10 @@ nodes
 | Test.java:105:33:105:53 | toPath(...) | semmle.label | toPath(...) |
 #select
 | Files.java:10:33:10:68 | getProperty(...) | Files.java:10:33:10:68 | getProperty(...) : String | Files.java:15:17:15:23 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Files.java:10:33:10:68 | getProperty(...) | system temp directory |
+| Test.java:23:33:23:68 | getProperty(...) | Test.java:23:33:23:68 | getProperty(...) : String | Test.java:24:59:24:65 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:23:33:23:68 | getProperty(...) | system temp directory |
+| Test.java:28:47:28:82 | getProperty(...) | Test.java:28:47:28:82 | getProperty(...) : String | Test.java:29:59:29:70 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:28:47:28:82 | getProperty(...) | system temp directory |
+| Test.java:33:33:33:68 | getProperty(...) | Test.java:33:33:33:68 | getProperty(...) : String | Test.java:34:59:34:65 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:33:33:33:68 | getProperty(...) | system temp directory |
+| Test.java:38:33:38:68 | getProperty(...) | Test.java:38:33:38:68 | getProperty(...) : String | Test.java:39:59:39:65 | tempDir | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:38:33:38:68 | getProperty(...) | system temp directory |
 | Test.java:53:38:53:73 | getProperty(...) | Test.java:53:38:53:73 | getProperty(...) : String | Test.java:54:9:54:20 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:53:38:53:73 | getProperty(...) | system temp directory |
 | Test.java:58:38:58:73 | getProperty(...) | Test.java:58:38:58:73 | getProperty(...) : String | Test.java:59:9:59:20 | tempDirChild | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:58:38:58:73 | getProperty(...) | system temp directory |
 | Test.java:63:38:63:73 | getProperty(...) | Test.java:63:38:63:73 | getProperty(...) : String | Test.java:64:21:64:41 | toPath(...) | Local information disclosure vulnerability from $@ due to use of file or directory readable by other local users. | Test.java:63:38:63:73 | getProperty(...) | system temp directory |