Merge branch 'main' into atorralba/java/open-redirect-sanitizer

java/ql/src/Security/CWE/CWE-022/TaintedPath.ql

@@ -18,21 +18,7 @@ import semmle.code.java.security.PathCreation
 import semmle.code.java.security.TaintedPathQuery
 import TaintedPathFlow::PathGraph
 
-/**
- * Gets the data-flow node at which to report a path ending at `sink`.
- *
- * Previously this query flagged alerts exclusively at `PathCreation` sites,
- * so to avoid perturbing existing alerts, where a `PathCreation` exists we
- * continue to report there; otherwise we report directly at `sink`.
- */
-DataFlow::Node getReportingNode(DataFlow::Node sink) {
-  TaintedPathFlow::flowTo(sink) and
-  if exists(PathCreation pc | pc.getAnInput() = sink.asExpr())
-  then result.asExpr() = any(PathCreation pc | pc.getAnInput() = sink.asExpr())
-  else result = sink
-}
-
 from TaintedPathFlow::PathNode source, TaintedPathFlow::PathNode sink
 where TaintedPathFlow::flowPath(source, sink)
-select getReportingNode(sink.getNode()), source, sink, "This path depends on a $@.",
-  source.getNode(), "user-provided value"
+select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
+  "user-provided value"

java/ql/src/Security/CWE/CWE-022/TaintedPathLocal.ql

@@ -18,21 +18,7 @@ import semmle.code.java.security.PathCreation
 import semmle.code.java.security.TaintedPathQuery
 import TaintedPathLocalFlow::PathGraph
 
-/**
- * Gets the data-flow node at which to report a path ending at `sink`.
- *
- * Previously this query flagged alerts exclusively at `PathCreation` sites,
- * so to avoid perturbing existing alerts, where a `PathCreation` exists we
- * continue to report there; otherwise we report directly at `sink`.
- */
-DataFlow::Node getReportingNode(DataFlow::Node sink) {
-  TaintedPathLocalFlow::flowTo(sink) and
-  if exists(PathCreation pc | pc.getAnInput() = sink.asExpr())
-  then result.asExpr() = any(PathCreation pc | pc.getAnInput() = sink.asExpr())
-  else result = sink
-}
-
 from TaintedPathLocalFlow::PathNode source, TaintedPathLocalFlow::PathNode sink
 where TaintedPathLocalFlow::flowPath(source, sink)
-select getReportingNode(sink.getNode()), source, sink, "This path depends on a $@.",
-  source.getNode(), "user-provided value"
+select sink.getNode(), source, sink, "This path depends on a $@.", source.getNode(),
+  "user-provided value"

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

@@ -25,7 +25,11 @@ import semmle.code.java.security.TempDirLocalInformationDisclosureQuery
  * resulting in a zero-length paths.
  */
 module InsecureMethodPathGraph implements DataFlow::PathGraphSig<MethodCallInsecureFileCreation> {
-  predicate edges(MethodCallInsecureFileCreation n1, MethodCallInsecureFileCreation n2) { none() }
+  predicate edges(
+    MethodCallInsecureFileCreation n1, MethodCallInsecureFileCreation n2, string key, string value
+  ) {
+    none()
+  }
 
   predicate nodes(MethodCallInsecureFileCreation n, string key, string val) {
     key = "semmle.label" and val = n.toString()

java/ql/src/change-notes/2024-01-26-path-injection-precision-improved.md

@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* The sinks of the queries `java/path-injection` and `java/path-injection-local` have been reworked. Path creation sinks have been converted to summaries instead, while sinks now are actual file read/write operations only. This has reduced the false positive ratio of both queries.

java/ql/src/experimental/Security/CWE/CWE-073/FilePathInjection.ql

@@ -16,6 +16,7 @@ import java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.security.TaintedPathQuery
 import JFinalController
 import semmle.code.java.security.PathSanitizer
 private import semmle.code.java.security.Sanitizers
@@ -52,7 +53,7 @@ module InjectFilePathConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source instanceof ThreatModelFlowSource }
 
   predicate isSink(DataFlow::Node sink) {
-    sinkNode(sink, "path-injection") and
+    sink instanceof TaintedPathSink and
     not sink instanceof NormalizedPathNode
   }