Merge pull request #8032 from JLLeitschuh/feat/JLL/check_os

Java: Add Guard Classes for checking OS & unify System Property Access

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

@@ -11,6 +11,7 @@
  */
 
 import java
+import semmle.code.java.os.OSCheck
 import TempDirUtils
 import DataFlow::PathGraph
 import semmle.code.java.dataflow.TaintTracking2
@@ -102,11 +103,36 @@ private class FileCreateTempFileSink extends FileCreationSink {
   }
 }
 
+/**
+ * A guard that holds when the program is definitely running under some version of Windows.
+ */
+abstract private class WindowsOsBarrierGuard extends DataFlow::BarrierGuard { }
+
+private class IsNotUnixBarrierGuard extends WindowsOsBarrierGuard instanceof IsUnixGuard {
+  override predicate checks(Expr e, boolean branch) {
+    this.controls(e.getBasicBlock(), branch.booleanNot())
+  }
+}
+
+private class IsWindowsBarrierGuard extends WindowsOsBarrierGuard instanceof IsWindowsGuard {
+  override predicate checks(Expr e, boolean branch) { this.controls(e.getBasicBlock(), branch) }
+}
+
+private class IsSpecificWindowsBarrierGuard extends WindowsOsBarrierGuard instanceof IsSpecificWindowsVariant {
+  override predicate checks(Expr e, boolean branch) {
+    branch = true and this.controls(e.getBasicBlock(), branch)
+  }
+}
+
+/**
+ * A taint tracking configuration tracking the access of the system temporary directory
+ * flowing to the creation of files or directories.
+ */
 private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Configuration {
   TempDirSystemGetPropertyToCreateConfig() { this = "TempDirSystemGetPropertyToCreateConfig" }
 
   override predicate isSource(DataFlow::Node source) {
-    source.asExpr() instanceof MethodAccessSystemGetPropertyTempDirTainted
+    source.asExpr() instanceof ExprSystemGetPropertyTempDirTainted
   }
 
   /**
@@ -129,6 +155,10 @@ private class TempDirSystemGetPropertyToCreateConfig extends TaintTracking::Conf
       sanitizer.asExpr() = sanitisingMethodAccess.getArgument(0)
     )
   }
+
+  override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
+    guard instanceof WindowsOsBarrierGuard
+  }
 }
 
 /**
@@ -147,10 +177,8 @@ private class TempDirSystemGetPropertyDirectlyToMkdirConfig extends TaintTrackin
   }
 
   override predicate isSource(DataFlow::Node node) {
-    exists(
-      MethodAccessSystemGetPropertyTempDirTainted propertyGetMethodAccess, DataFlow::Node callSite
-    |
-      DataFlow::localFlow(DataFlow::exprNode(propertyGetMethodAccess), callSite)
+    exists(ExprSystemGetPropertyTempDirTainted propertyGetExpr, DataFlow::Node callSite |
+      DataFlow::localFlow(DataFlow::exprNode(propertyGetExpr), callSite)
     |
       isFileConstructorArgument(callSite.asExpr(), node.asExpr(), 1)
     )

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

@@ -1,10 +1,14 @@
 import java.io.File;
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.nio.file.Files;
+import java.nio.file.Path;
 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-------`
@@ -30,7 +34,7 @@ public class TempDirUsageSafe {
         createTempFile(tempChildFile.toPath()); // GOOD: Good has permissions `-rw-------`
     }
 
-    static void createTempFile(Path tempDir) {
+    static void createTempFile(Path tempDirChild) {
         try {
             if (tempDirChild.getFileSystem().supportedFileAttributeViews().contains("posix")) {
                 // Explicit permissions setting is only required on unix-like systems because

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

@@ -3,34 +3,14 @@
  */
 
 import java
+private import semmle.code.java.environment.SystemProperty
 import semmle.code.java.dataflow.FlowSources
 
 /**
- * A method that returns a `String` or `File` that has been tainted by `System.getProperty("java.io.tmpdir")`.
+ * A method or field access 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
-    )
-  }
+class ExprSystemGetPropertyTempDirTainted extends Expr {
+  ExprSystemGetPropertyTempDirTainted() { this = getSystemProperty("java.io.tmpdir") }
 }
 
 /**