Add an implementation of TaintedPathATM.qll and corresponding positive EndpointCharacteristic in Java

java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/EndpointCharacteristics.qll

@@ -5,9 +5,12 @@
 private import java as java
 import semmle.code.java.dataflow.TaintTracking
 import semmle.code.java.security.QueryInjection
+import semmle.code.java.security.PathCreation
+private import semmle.code.java.dataflow.ExternalFlow
 import experimental.adaptivethreatmodeling.EndpointTypes
 private import experimental.adaptivethreatmodeling.ATMConfig
 private import experimental.adaptivethreatmodeling.SqlInjectionATM
+private import experimental.adaptivethreatmodeling.TaintedPathATM
 private import semmle.code.java.security.ExternalAPIs as ExternalAPIs
 private import semmle.code.java.Expr as Expr
 
@@ -155,11 +158,11 @@ abstract class EndpointCharacteristic extends string {
 /*
  * Characteristics that are indicative of a sink.
  * NOTE: Initially each sink type has only one characteristic, which is that it's a sink of this type in the standard
- * JavaScript libraries.
+ * Java libraries.
  */
 
 // /**
-//  * Endpoints identified as "DomBasedXssSink" by the standard JavaScript libraries are XSS sinks with maximal confidence.
+//  * Endpoints identified as "DomBasedXssSink" by the standard Java libraries are XSS sinks with maximal confidence.
 //  */
 // private class DomBasedXssSinkCharacteristic extends EndpointCharacteristic {
 //   DomBasedXssSinkCharacteristic() { this = "DomBasedXssSink" }
@@ -172,23 +175,30 @@ abstract class EndpointCharacteristic extends string {
 //     confidence = maximalConfidence()
 //   }
 // }
-// /**
-//  * Endpoints identified as "TaintedPathSink" by the standard JavaScript libraries are path injection sinks with maximal
-//  * confidence.
-//  */
-// private class TaintedPathSinkCharacteristic extends EndpointCharacteristic {
-//   TaintedPathSinkCharacteristic() { this = "TaintedPathSink" }
-//   override predicate appliesToEndpoint(DataFlow::Node n) { n instanceof TaintedPath::Sink }
-//   override predicate hasImplications(
-//     EndpointType endpointClass, boolean isPositiveIndicator, float confidence
-//   ) {
-//     endpointClass instanceof TaintedPathSinkType and
-//     isPositiveIndicator = true and
-//     confidence = maximalConfidence()
-//   }
-// }
 /**
- * Endpoints identified as "SqlInjectionSink" by the standard JavaScript libraries are SQL injection sinks with maximal
+ * Endpoints identified as "TaintedPathSink" by the standard Java libraries are path injection sinks with maximal
+ * confidence.
+ */
+private class TaintedPathSinkCharacteristic extends EndpointCharacteristic {
+  TaintedPathSinkCharacteristic() { this = "TaintedPathSink" }
+
+  override predicate appliesToEndpoint(DataFlow::Node n) {
+    n.asExpr() = any(PathCreation p).getAnInput()
+    or
+    sinkNode(n, "create-file")
+  }
+
+  override predicate hasImplications(
+    EndpointType endpointClass, boolean isPositiveIndicator, float confidence
+  ) {
+    endpointClass instanceof TaintedPathSinkType and
+    isPositiveIndicator = true and
+    confidence = maximalConfidence()
+  }
+}
+
+/**
+ * Endpoints identified as "SqlInjectionSink" by the standard Java libraries are SQL injection sinks with maximal
  * confidence.
  */
 private class SqlInjectionSinkCharacteristic extends EndpointCharacteristic {
@@ -206,7 +216,7 @@ private class SqlInjectionSinkCharacteristic extends EndpointCharacteristic {
 }
 
 // /**
-//  * Endpoints identified as "NosqlInjectionSink" by the standard JavaScript libraries are NoSQL injection sinks with
+//  * Endpoints identified as "NosqlInjectionSink" by the standard Java libraries are NoSQL injection sinks with
 //  * maximal confidence.
 //  */
 // private class NosqlInjectionSinkCharacteristic extends EndpointCharacteristic {

java/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/TaintedPathATM.qll

@@ -3,16 +3,18 @@
  *
  * A taint-tracking configuration for reasoning about path injection vulnerabilities.
  * Defines shared code used by the path injection boosted query.
+ * Largely copied from java/ql/src/Security/CWE/CWE-022/TaintedPath.ql.
  */
 
-import semmle.javascript.heuristics.SyntacticHeuristics
-import semmle.javascript.security.dataflow.TaintedPathCustomizations
+import java
+import semmle.code.java.security.PathSanitizer
 import AdaptiveThreatModeling
+import semmle.code.java.dataflow.FlowSources
 
 class TaintedPathAtmConfig extends AtmConfig {
   TaintedPathAtmConfig() { this = "TaintedPathAtmConfig" }
 
-  override predicate isKnownSource(DataFlow::Node source) { source instanceof TaintedPath::Source }
+  override predicate isKnownSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 
   override EndpointType getASinkEndpointType() { result instanceof TaintedPathSinkType }
 
@@ -21,44 +23,60 @@ class TaintedPathAtmConfig extends AtmConfig {
    * query, except additional ATM sinks have been added to the `isSink` predicate.
    */
 
-  override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
-    label = sink.(TaintedPath::Sink).getAFlowLabel()
-    or
-    // Allow effective sinks to have any taint label
-    isEffectiveSink(sink)
+  override predicate isSanitizer(DataFlow::Node sanitizer) {
+    sanitizer.getType() instanceof BoxedType or
+    sanitizer.getType() instanceof PrimitiveType or
+    sanitizer.getType() instanceof NumberType or
+    sanitizer instanceof PathInjectionSanitizer
   }
 
-  override predicate isSanitizer(DataFlow::Node node) { node instanceof TaintedPath::Sanitizer }
-
-  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode node) {
-    node instanceof BarrierGuardNodeAsSanitizerGuardNode
+  override predicate isAdditionalTaintStep(DataFlow::Node n1, DataFlow::Node n2) {
+    any(TaintedPathAdditionalTaintStep s).step(n1, n2)
+  }
   }
 
-  override predicate isAdditionalFlowStep(
-    DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel,
-    DataFlow::FlowLabel dstlabel
-  ) {
-    TaintedPath::isAdditionalTaintedPathFlowStep(src, dst, srclabel, dstlabel)
-  }
-}
+/*
+ * Models a very basic guard for the tainted path queries.
+ * TODO: Copied from java/ql/src/Security/CWE/CWE-022/TaintedPathCommon.qll because I couldn't figure out how to import it.
+ */
 
 /**
- * This class provides sanitizer guards for path injection.
+ * A unit class for adding additional taint steps.
  *
- * The standard library path injection query uses a data flow configuration, and therefore defines
- * barrier nodes. However we're using a taint tracking configuration for path injection to find new
- * kinds of less certain results. Since taint tracking configurations use sanitizer guards instead
- * of barrier guards, we port the barrier guards for the boosted query from the standard library to
- * sanitizer guards here.
+ * Extend this class to add additional taint steps that should apply to tainted path flow configurations.
  */
-private class BarrierGuardNodeAsSanitizerGuardNode extends TaintTracking::LabeledSanitizerGuardNode {
-  BarrierGuardNodeAsSanitizerGuardNode() { this instanceof TaintedPath::BarrierGuardNode }
+class TaintedPathAdditionalTaintStep extends Unit {
+  abstract predicate step(DataFlow::Node n1, DataFlow::Node n2);
+}
 
-  override predicate sanitizes(boolean outcome, Expr e) {
-    blocks(outcome, e) or blocks(outcome, e, _)
+private class DefaultTaintedPathAdditionalTaintStep extends TaintedPathAdditionalTaintStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(Argument a |
+      a = n1.asExpr() and
+      a.getCall() = n2.asExpr() and
+      a = any(TaintPreservingUriCtorParam tpp).getAnArgument()
+    )
+  }
   }
 
-  override predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
-    sanitizes(outcome, e) and exists(label)
+private class TaintPreservingUriCtorParam extends Parameter {
+  TaintPreservingUriCtorParam() {
+    exists(Constructor ctor, int idx, int nParams |
+      ctor.getDeclaringType() instanceof TypeUri and
+      this = ctor.getParameter(idx) and
+      nParams = ctor.getNumberOfParameters()
+    |
+      // URI(String scheme, String ssp, String fragment)
+      idx = 1 and nParams = 3
+      or
+      // URI(String scheme, String host, String path, String fragment)
+      idx = [1, 2] and nParams = 4
+      or
+      // URI(String scheme, String authority, String path, String query, String fragment)
+      idx = 2 and nParams = 5
+      or
+      // URI(String scheme, String userInfo, String host, int port, String path, String query, String fragment)
+      idx = 4 and nParams = 7
+    )
   }
 }

java/ql/experimental/adaptivethreatmodeling/src/ExtractPositiveExamples.ql

@@ -12,6 +12,7 @@ import semmle.code.java.dataflow.TaintTracking
 private import experimental.adaptivethreatmodeling.EndpointCharacteristics as EndpointCharacteristics
 private import experimental.adaptivethreatmodeling.ATMConfig as AtmConfig
 private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm
+private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 
 from
   DataFlow::Node sink, AtmConfig::AtmConfig config,
@@ -20,4 +21,4 @@ where
   characteristic.appliesToEndpoint(sink) and
   confidence >= characteristic.maximalConfidence() and
   characteristic.hasImplications(config.getASinkEndpointType(), true, confidence)
-select sink, "Sink of type " + characteristic + " with confidence " + confidence.toString()
+select sink, characteristic.toString()

java/ql/experimental/adaptivethreatmodeling/src/ExtractSinkCandidatesWithFlow.ql

@@ -4,15 +4,10 @@
  *
  * Note: This query does not actually classify the endpoints using the model.
  *
- * TODO: Produce CSV/JSON/SARIF output describing these endpoints (probably just a URL for each endpoint).
- *
- * @name SQL database query built from user-controlled sources (experimental)
- * @description Building a database query from user-controlled sources is vulnerable to insertion of
- *              malicious code by the user.
+ * @name Sink candidates with flow (experimental)
+ * @description Sink candidates with flow from a source
  * @kind problem
- * @problem.severity error
- * @security-severity 8.8
- * @id java/ml-powered/sql-injection
+ * @id java/ml-powered/sink-candidates-with-flow
  * @tags experimental security
  */
 
@@ -21,10 +16,19 @@ import semmle.code.java.dataflow.TaintTracking
 private import experimental.adaptivethreatmodeling.ATMConfig as AtmConfig
 // private import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAtm
 private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm
+private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 
-// private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 // private import experimental.adaptivethreatmodeling.XssATM as XssAtm
 // private import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
-from DataFlow::PathNode sink
-where exists(AtmConfig::AtmConfig queryConfig | queryConfig.isSinkCandidateWithFlow(sink))
-select sink.getNode(), "SQL injection sink candidate"
+from DataFlow::PathNode sink, string message
+where
+  // The message is the concatenation of all relevant configs
+  message =
+    concat(AtmConfig::AtmConfig config |
+      config.isSinkCandidateWithFlow(sink)
+    |
+      config.getASinkEndpointType().getDescription() + ", "
+      order by
+        config.getASinkEndpointType().getDescription()
+    )
+select sink.getNode(), message