Fix inverted predicate logic and add additional test cases

github · Jan 19, 2022 · f5dfc81 · f5dfc81
1 parent 6854bba
commit f5dfc81
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 7 deletions.
diff --git a/java/ql/src/Security/CWE/CWE-378/TempDirHijackingVulnerability.ql b/java/ql/src/Security/CWE/CWE-378/TempDirHijackingVulnerability.ql
@@ -79,12 +79,12 @@ predicate isSinkConstrainedByIfCheck(DataFlow2::Node sink) {
 }
 
 from
-  DataFlow::Node source, DataFlow::Node deleteCheckpoint, DataFlow2::PathNode deleteCheckpoint2,
-  DataFlow2::PathNode sink, TempDirHijackingToDeleteConfig toDeleteConfig,
+  DataFlow::PathNode source, DataFlow::PathNode deleteCheckpoint, DataFlow2::Node deleteCheckpoint2,
+  DataFlow2::Node sink, TempDirHijackingToDeleteConfig toDeleteConfig,
   TempDirHijackingFromDeleteConfig fromDeleteConfig
 where
-  toDeleteConfig.hasFlow(source, deleteCheckpoint) and
-  fromDeleteConfig.hasFlowPath(deleteCheckpoint2, sink) and
-  deleteCheckpoint.asExpr() = deleteCheckpoint2.getNode().asExpr() and
-  isSinkConstrainedByIfCheck(sink.getNode())
-select deleteCheckpoint2, deleteCheckpoint2, sink, "TODO %", sink
+  toDeleteConfig.hasFlowPath(source, deleteCheckpoint) and
+  fromDeleteConfig.hasFlow(deleteCheckpoint2, sink) and
+  deleteCheckpoint.getNode().asExpr() = deleteCheckpoint2.asExpr() and
+  not isSinkConstrainedByIfCheck(sink)
+select deleteCheckpoint.getNode(), source, deleteCheckpoint, "Local temporary directory hijacking race condition $@", sink, "here"
diff --git a/java/ql/test/query-tests/security/CWE-378/semmle/tests/Test.java b/java/ql/test/query-tests/security/CWE-378/semmle/tests/Test.java
@@ -35,5 +35,33 @@ static File safe2() {
             throw new RuntimeException("Failed to create directory");
         }
     }
+
+    static File safe3() {
+        File temp = File.createTempFile("test", "directory");
+        temp.delete();
+        if (!(temp.mkdirs()))) {
+            throw new RuntimeException("Failed to create directory");
+        }
+        return temp;
+    }
+
+    static File safe4() {
+        boolean success = true;
+        File temp = File.createTempFile("test", "directory");
+        success &= temp.delete();
+        success &= f.mkdir();
+        if (!success) {
+            throw new RuntimeException("Failed to create directory");
+        }
+    }
+
+    static File safe5() {
+        File temp = File.createTempFile("test", "directory");
+        if (temp.delete() && temp.mkdir()) {
+            return temp;
+        } else {
+            throw new RuntimeException("Failed to create directory");
+        }
+    }
 
 }