github · tiferet · Nov 30, 2022 · Nov 17, 2022 · Nov 17, 2022 · Nov 18, 2022
@@ -7,6 +7,7 @@
 private import javascript as JS
 import EndpointTypes
 import EndpointCharacteristics as EndpointCharacteristics
+import AdaptiveThreatModeling::ATM::ResultsInfo as AtmResultsInfo
 
 /**
  * EXPERIMENTAL. This API may change in the future.
@@ -29,10 +30,23 @@ import EndpointCharacteristics as EndpointCharacteristics
  * `isAdditionalFlowStep` with a more generalised definition of additional edges. See
  * `NosqlInjectionATM.qll` for an example of doing this.
  */
-abstract class AtmConfig extends string {
+abstract class AtmConfig extends JS::TaintTracking::Configuration {
   bindingset[this]
   AtmConfig() { any() }
 
+  /**
+   * Holds if `source` is a relevant taint source. When sources are not boosted, `isSource` is equivalent to
+   * `isKnownSource` (i.e there are no "effective" sources to be classified by an ML model).
+   */
+  override predicate isSource(JS::DataFlow::Node source) { this.isKnownSource(source) }
+
+  /**
+   * Holds if `sink` is a known taint sink or an "effective" sink (a candidate to be classified by an ML model).
+   */
+  override predicate isSink(JS::DataFlow::Node sink) {
+    this.isKnownSink(sink) or this.isEffectiveSink(sink)
+  }
+
   /**
    * EXPERIMENTAL. This API may change in the future.
    *
@@ -127,6 +141,17 @@ abstract class AtmConfig extends string {
    * A cut-off value of 1 produces all alerts including those that are likely false-positives.
    */
   float getScoreCutoff() { result = 0.0 }
+
+  /**
+   * Holds if there's an ATM alert (a flow path from `source` to `sink` with ML-determined likelihood `score`) according
+   * to this ML-boosted configuration, whereas the unboosted base query does not contain this source and sink
+   * combination.
+   */
+  predicate getAlerts(JS::DataFlow::PathNode source, JS::DataFlow::PathNode sink, float score) {
+    this.hasFlowPath(source, sink) and
+    not AtmResultsInfo::isFlowLikelyInBaseQuery(source.getNode(), sink.getNode()) and
+    score = AtmResultsInfo::getScoreForFlow(source.getNode(), sink.getNode())
+  }
 }
 
 /** DEPRECATED: Alias for AtmConfig */

@@ -1,6 +1,7 @@
 /**
  * For internal use only.
  *
+ * A taint-tracking configuration for reasoning about NoSQL injection vulnerabilities.
  * Defines shared code used by the NoSQL injection boosted query.
  */
 
@@ -10,62 +11,19 @@ private import semmle.javascript.security.dataflow.NosqlInjectionCustomizations
 import AdaptiveThreatModeling
 
 class NosqlInjectionAtmConfig extends AtmConfig {
-  NosqlInjectionAtmConfig() { this = "NosqlInjectionATMConfig" }
+  NosqlInjectionAtmConfig() { this = "NosqlInjectionAtmConfig" }
 
   override predicate isKnownSource(DataFlow::Node source) {
     source instanceof NosqlInjection::Source or TaintedObject::isSource(source, _)
   }
 
   override EndpointType getASinkEndpointType() { result instanceof NosqlInjectionSinkType }
-}
-
-/** DEPRECATED: Alias for NosqlInjectionAtmConfig */
-deprecated class NosqlInjectionATMConfig = NosqlInjectionAtmConfig;
-
-/** Holds if src -> trg is an additional flow step in the non-boosted NoSql injection security query. */
-predicate isBaseAdditionalFlowStep(
-  DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
-) {
-  TaintedObject::step(src, trg, inlbl, outlbl)
-  or
-  // additional flow step to track taint through NoSQL query objects
-  inlbl = TaintedObject::label() and
-  outlbl = TaintedObject::label() and
-  exists(NoSql::Query query, DataFlow::SourceNode queryObj |
-    queryObj.flowsTo(query) and
-    queryObj.flowsTo(trg) and
-    src = queryObj.getAPropertyWrite().getRhs()
-  )
-}
-
-/**
- * Gets a value that is (transitively) written to `query`, where `query` is a NoSQL sink.
- *
- * This predicate allows us to propagate data flow through property writes and array constructors
- * within a query object, enabling the security query to pick up NoSQL injection vulnerabilities
- * involving more complex queries.
- */
-DataFlow::Node getASubexpressionWithinQuery(DataFlow::Node query) {
-  any(NosqlInjectionAtmConfig cfg).isEffectiveSink(query) and
-  exists(DataFlow::SourceNode receiver |
-    receiver = [getASubexpressionWithinQuery(query), query].getALocalSource()
-  |
-    result =
-      [receiver.getAPropertyWrite().getRhs(), receiver.(DataFlow::ArrayCreationNode).getAnElement()]
-  )
-}
-
-/**
- * A taint-tracking configuration for reasoning about NoSQL injection vulnerabilities.
- *
- * This is largely a copy of the taint tracking configuration for the standard NoSQL injection
- * query, except additional ATM sinks have been added and the additional flow step has been
- * generalised to cover the sinks predicted by ATM.
- */
-class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "NosqlInjectionATM" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof NosqlInjection::Source }
+  /*
+   * This is largely a copy of the taint tracking configuration for the standard NoSQL injection
+   * query, except additional ATM sinks have been added and the additional flow step has been
+   * generalised to cover the sinks predicted by ATM.
+   */
 
   override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
     TaintedObject::isSource(source, label)
@@ -75,7 +33,7 @@ class Configuration extends TaintTracking::Configuration {
     sink.(NosqlInjection::Sink).getAFlowLabel() = label
     or
     // Allow effective sinks to have any taint label
-    any(NosqlInjectionAtmConfig cfg).isEffectiveSink(sink)
+    isEffectiveSink(sink)
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
@@ -94,7 +52,43 @@ class Configuration extends TaintTracking::Configuration {
     isBaseAdditionalFlowStep(src, trg, inlbl, outlbl)
     or
     // relaxed version of previous step to track taint through unmodeled NoSQL query objects
-    any(NosqlInjectionAtmConfig cfg).isEffectiveSink(trg) and
+    isEffectiveSink(trg) and
     src = getASubexpressionWithinQuery(trg)
   }
+
+  /** Holds if src -> trg is an additional flow step in the non-boosted NoSql injection security query. */
+  private predicate isBaseAdditionalFlowStep(
+    DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
+  ) {
+    TaintedObject::step(src, trg, inlbl, outlbl)
+    or
+    // additional flow step to track taint through NoSQL query objects
+    inlbl = TaintedObject::label() and
+    outlbl = TaintedObject::label() and
+    exists(NoSql::Query query, DataFlow::SourceNode queryObj |
+      queryObj.flowsTo(query) and
+      queryObj.flowsTo(trg) and
+      src = queryObj.getAPropertyWrite().getRhs()
+    )
+  }
+
+  /**
+   * Gets a value that is (transitively) written to `query`, where `query` is a NoSQL sink.
+   *
+   * This predicate allows us to propagate data flow through property writes and array constructors
+   * within a query object, enabling the security query to pick up NoSQL injection vulnerabilities
+   * involving more complex queries.
+   */
+  private DataFlow::Node getASubexpressionWithinQuery(DataFlow::Node query) {
+    isEffectiveSink(query) and
+    exists(DataFlow::SourceNode receiver |
+      receiver = [getASubexpressionWithinQuery(query), query].getALocalSource()
+    |
+      result =
+        [
+          receiver.getAPropertyWrite().getRhs(),
+          receiver.(DataFlow::ArrayCreationNode).getAnElement()
+        ]
+    )
+  }
 }
@@ -1,6 +1,7 @@
 /**
  * For internal use only.
  *
+ * A taint-tracking configuration for reasoning about SQL injection vulnerabilities.
  * Defines shared code used by the SQL injection boosted query.
  */
 
@@ -9,30 +10,16 @@ import semmle.javascript.security.dataflow.SqlInjectionCustomizations
 import AdaptiveThreatModeling
 
 class SqlInjectionAtmConfig extends AtmConfig {
-  SqlInjectionAtmConfig() { this = "SqlInjectionATMConfig" }
+  SqlInjectionAtmConfig() { this = "SqlInjectionAtmConfig" }
 
   override predicate isKnownSource(DataFlow::Node source) { source instanceof SqlInjection::Source }
 
   override EndpointType getASinkEndpointType() { result instanceof SqlInjectionSinkType }
-}
-
-/** DEPRECATED: Alias for SqlInjectionAtmConfig */
-deprecated class SqlInjectionATMConfig = SqlInjectionAtmConfig;
-
-/**
- * A taint-tracking configuration for reasoning about SQL injection vulnerabilities.
- *
- * This is largely a copy of the taint tracking configuration for the standard SQL injection
- * query, except additional sinks have been added using the sink endpoint filter.
- */
-class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "SqlInjectionATM" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof SqlInjection::Source }
-
-  override predicate isSink(DataFlow::Node sink) {
-    sink instanceof SqlInjection::Sink or any(SqlInjectionAtmConfig cfg).isEffectiveSink(sink)
-  }
+  /*
+   * This is largely a copy of the taint tracking configuration for the standard SQL injection
+   * query, except additional sinks have been added using the sink endpoint filter.
+   */
 
   override predicate isSanitizer(DataFlow::Node node) {
     super.isSanitizer(node) or

@@ -1,6 +1,7 @@
 /**
  * For internal use only.
  *
+ * A taint-tracking configuration for reasoning about path injection vulnerabilities.
  * Defines shared code used by the path injection boosted query.
  */
 
@@ -9,32 +10,22 @@ import semmle.javascript.security.dataflow.TaintedPathCustomizations
 import AdaptiveThreatModeling
 
 class TaintedPathAtmConfig extends AtmConfig {
-  TaintedPathAtmConfig() { this = "TaintedPathATMConfig" }
+  TaintedPathAtmConfig() { this = "TaintedPathAtmConfig" }
 
   override predicate isKnownSource(DataFlow::Node source) { source instanceof TaintedPath::Source }
 
   override EndpointType getASinkEndpointType() { result instanceof TaintedPathSinkType }
-}
-
-/** DEPRECATED: Alias for TaintedPathAtmConfig */
-deprecated class TaintedPathATMConfig = TaintedPathAtmConfig;
-
-/**
- * A taint-tracking configuration for reasoning about path injection vulnerabilities.
- *
- * This is largely a copy of the taint tracking configuration for the standard path injection
- * query, except additional ATM sinks have been added to the `isSink` predicate.
- */
-class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "TaintedPathATM" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof TaintedPath::Source }
+  /*
+   * This is largely a copy of the taint tracking configuration for the standard path injection
+   * 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
-    any(TaintedPathAtmConfig cfg).isEffectiveSink(sink)
+    isEffectiveSink(sink)
   }
 
   override predicate isSanitizer(DataFlow::Node node) { node instanceof TaintedPath::Sanitizer }
@@ -60,7 +51,7 @@ class Configuration extends TaintTracking::Configuration {
  * of barrier guards, we port the barrier guards for the boosted query from the standard library to
  * sanitizer guards here.
  */
-class BarrierGuardNodeAsSanitizerGuardNode extends TaintTracking::LabeledSanitizerGuardNode {
+private class BarrierGuardNodeAsSanitizerGuardNode extends TaintTracking::LabeledSanitizerGuardNode {
   BarrierGuardNodeAsSanitizerGuardNode() { this instanceof TaintedPath::BarrierGuardNode }
 
   override predicate sanitizes(boolean outcome, Expr e) {

@@ -1,6 +1,7 @@
 /**
  * For internal use only.
  *
+ * A taint-tracking configuration for reasoning about XSS vulnerabilities.
  * Defines shared code used by the XSS boosted query.
  */
 
@@ -9,31 +10,16 @@ private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
 import AdaptiveThreatModeling
 
 class DomBasedXssAtmConfig extends AtmConfig {
-  DomBasedXssAtmConfig() { this = "DomBasedXssATMConfig" }
+  DomBasedXssAtmConfig() { this = "DomBasedXssAtmConfig" }
 
   override predicate isKnownSource(DataFlow::Node source) { source instanceof DomBasedXss::Source }
 
   override EndpointType getASinkEndpointType() { result instanceof XssSinkType }
-}
-
-/** DEPRECATED: Alias for DomBasedXssAtmConfig */
-deprecated class DomBasedXssATMConfig = DomBasedXssAtmConfig;
-
-/**
- * A taint-tracking configuration for reasoning about XSS vulnerabilities.
- *
- * This is largely a copy of the taint tracking configuration for the standard XSSThroughDom query,
- * except additional ATM sinks have been added to the `isSink` predicate.
- */
-class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "DomBasedXssATMConfiguration" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof DomBasedXss::Source }
-
-  override predicate isSink(DataFlow::Node sink) {
-    sink instanceof DomBasedXss::Sink or
-    any(DomBasedXssAtmConfig cfg).isEffectiveSink(sink)
-  }
+  /*
+   * This is largely a copy of the taint tracking configuration for the standard XSSThroughDom query,
+   * except additional ATM sinks have been added to the `isSink` predicate.
+   */
 
   override predicate isSanitizer(DataFlow::Node node) {
     super.isSanitizer(node) or

@@ -206,13 +206,14 @@ query predicate reformattedTrainingEndpoints(
  * TODO: Delete this once we are no longer surfacing `hasFlowFromSource`.
  */
 DataFlow::Configuration getDataFlowCfg(Query query) {
-  query instanceof NosqlInjectionQuery and result instanceof NosqlInjectionAtm::Configuration
+  query instanceof NosqlInjectionQuery and
+  result instanceof NosqlInjectionAtm::NosqlInjectionAtmConfig
   or
-  query instanceof SqlInjectionQuery and result instanceof SqlInjectionAtm::Configuration
+  query instanceof SqlInjectionQuery and result instanceof SqlInjectionAtm::SqlInjectionAtmConfig
   or
-  query instanceof TaintedPathQuery and result instanceof TaintedPathAtm::Configuration
+  query instanceof TaintedPathQuery and result instanceof TaintedPathAtm::TaintedPathAtmConfig
   or
-  query instanceof XssQuery and result instanceof XssAtm::Configuration
+  query instanceof XssQuery and result instanceof XssAtm::DomBasedXssAtmConfig
 }
 
 // TODO: Delete this once we are no longer surfacing `hasFlowFromSource`.

@@ -17,11 +17,8 @@ import ATM::ResultsInfo
 import DataFlow::PathGraph
 import experimental.adaptivethreatmodeling.NosqlInjectionATM
 
-from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
-where
-  cfg.hasFlowPath(source, sink) and
-  not isFlowLikelyInBaseQuery(source.getNode(), sink.getNode()) and
-  score = getScoreForFlow(source.getNode(), sink.getNode())
+from AtmConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
+where cfg.getAlerts(source, sink, score)
 select sink.getNode(), source, sink,
   "(Experimental) This may be a database query that depends on $@. Identified using machine learning.",
   source.getNode(), "a user-provided value", score
@@ -17,11 +17,8 @@ import experimental.adaptivethreatmodeling.SqlInjectionATM
 import ATM::ResultsInfo
 import DataFlow::PathGraph
 
-from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
-where
-  cfg.hasFlowPath(source, sink) and
-  not isFlowLikelyInBaseQuery(source.getNode(), sink.getNode()) and
-  score = getScoreForFlow(source.getNode(), sink.getNode())
+from AtmConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
+where cfg.getAlerts(source, sink, score)
 select sink.getNode(), source, sink,
   "(Experimental) This may be a database query that depends on $@. Identified using machine learning.",
   source.getNode(), "a user-provided value", score
@@ -21,11 +21,8 @@ import ATM::ResultsInfo
 import DataFlow::PathGraph
 import experimental.adaptivethreatmodeling.TaintedPathATM
 
-from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
-where
-  cfg.hasFlowPath(source, sink) and
-  not isFlowLikelyInBaseQuery(source.getNode(), sink.getNode()) and
-  score = getScoreForFlow(source.getNode(), sink.getNode())
+from AtmConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
+where cfg.getAlerts(source, sink, score)
 select sink.getNode(), source, sink,
   "(Experimental) This may be a path that depends on $@. Identified using machine learning.",
   source.getNode(), "a user-provided value", score