JS: Port SqlInjection

javascript/ql/lib/semmle/javascript/security/dataflow/NosqlInjectionQuery.qll

@@ -14,7 +14,57 @@ import NosqlInjectionCustomizations::NosqlInjection
 /**
  * A taint-tracking configuration for reasoning about SQL-injection vulnerabilities.
  */
-class Configuration extends TaintTracking::Configuration {
+module NosqlInjectionConfig implements DataFlow::StateConfigSig {
+  class FlowState = DataFlow::FlowLabel;
+
+  predicate isSource(DataFlow::Node source, DataFlow::FlowLabel state) {
+    source instanceof Source and state.isTaint()
+    or
+    TaintedObject::isSource(source, state)
+  }
+
+  predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel state) {
+    sink.(Sink).getAFlowLabel() = state
+  }
+
+  predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel state) {
+    node instanceof Sanitizer and state.isTaint()
+    or
+    TaintTracking::defaultSanitizer(node) and state.isTaint()
+    or
+    node = TaintedObject::SanitizerGuard::getABarrierNode(state)
+  }
+
+  predicate isAdditionalFlowStep(
+    DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2,
+    DataFlow::FlowLabel state2
+  ) {
+    TaintedObject::step(node1, node2, state1, state2)
+    or
+    // additional flow step to track taint through NoSQL query objects
+    state1 = TaintedObject::label() and
+    state2 = TaintedObject::label() and
+    exists(NoSql::Query query, DataFlow::SourceNode queryObj |
+      queryObj.flowsTo(query) and
+      queryObj.flowsTo(node2) and
+      node1 = queryObj.getAPropertyWrite().getRhs()
+    )
+    or
+    TaintTracking::defaultTaintStep(node1, node2) and
+    state1.isTaint() and
+    state2 = state1
+  }
+}
+
+/**
+ * Taint-tracking for reasoning about SQL-injection vulnerabilities.
+ */
+module NosqlInjectionFlow = DataFlow::GlobalWithState<NosqlInjectionConfig>;
+
+/**
+ * DEPRECATED. Use the `NosqlInjectionFlow` module instead.
+ */
+deprecated class Configuration extends TaintTracking::Configuration {
   Configuration() { this = "NosqlInjection" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -37,17 +87,9 @@ class Configuration extends TaintTracking::Configuration {
   }
 
   override predicate isAdditionalFlowStep(
-    DataFlow::Node src, DataFlow::Node trg, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
+    DataFlow::Node node1, DataFlow::Node node2, DataFlow::FlowLabel state1,
+    DataFlow::FlowLabel state2
   ) {
-    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()
-    )
+    NosqlInjectionConfig::isAdditionalFlowStep(node1, state1, node2, state2)
   }
 }

javascript/ql/lib/semmle/javascript/security/dataflow/SqlInjectionQuery.qll

@@ -13,7 +13,35 @@ import SqlInjectionCustomizations::SqlInjection
 /**
  * A taint-tracking configuration for reasoning about string based query injection vulnerabilities.
  */
-class Configuration extends TaintTracking::Configuration {
+module SqlInjectionConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+
+  predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(LdapJS::TaintPreservingLdapFilterStep filter |
+      pred = filter.getInput() and
+      succ = filter.getOutput()
+    )
+    or
+    exists(HtmlSanitizerCall call |
+      pred = call.getInput() and
+      succ = call
+    )
+  }
+}
+
+/**
+ * Taint-tracking for reasoning about string based query injection vulnerabilities.
+ */
+module SqlInjectionFlow = TaintTracking::Global<SqlInjectionConfig>;
+
+/**
+ * DEPRECATED. Use the `SqlInjectionFlow` module instead.
+ */
+deprecated class Configuration extends TaintTracking::Configuration {
   Configuration() { this = "SqlInjection" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -26,14 +54,6 @@ class Configuration extends TaintTracking::Configuration {
   }
 
   override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
-    exists(LdapJS::TaintPreservingLdapFilterStep filter |
-      pred = filter.getInput() and
-      succ = filter.getOutput()
-    )
-    or
-    exists(HtmlSanitizerCall call |
-      pred = call.getInput() and
-      succ = call
-    )
+    SqlInjectionConfig::isAdditionalFlowStep(pred, succ)
   }
 }

javascript/ql/src/Security/CWE-089/SqlInjection.ql

@@ -14,17 +14,23 @@
  */
 
 import javascript
-import semmle.javascript.security.dataflow.SqlInjectionQuery as SqlInjection
-import semmle.javascript.security.dataflow.NosqlInjectionQuery as NosqlInjection
-import DataFlow::PathGraph
+import semmle.javascript.security.dataflow.SqlInjectionQuery as Sql
+import semmle.javascript.security.dataflow.NosqlInjectionQuery as Nosql
 
-from DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, string type
+module Merged =
+  DataFlow::MergePathGraph<Sql::SqlInjectionFlow::PathNode, Nosql::NosqlInjectionFlow::PathNode,
+    Sql::SqlInjectionFlow::PathGraph, Nosql::NosqlInjectionFlow::PathGraph>;
+
+import DataFlow::DeduplicatePathGraph<Merged::PathNode, Merged::PathGraph>
+
+from PathNode source, PathNode sink, string type
 where
-  (
-    cfg instanceof SqlInjection::Configuration and type = "string"
-    or
-    cfg instanceof NosqlInjection::Configuration and type = "object"
-  ) and
-  cfg.hasFlowPath(source, sink)
+  Sql::SqlInjectionFlow::flowPath(source.getAnOriginalPathNode().asPathNode1(),
+    sink.getAnOriginalPathNode().asPathNode1()) and
+  type = "string"
+  or
+  Nosql::NosqlInjectionFlow::flowPath(source.getAnOriginalPathNode().asPathNode2(),
+    sink.getAnOriginalPathNode().asPathNode2()) and
+  type = "object"
 select sink.getNode(), source, sink, "This query " + type + " depends on a $@.", source.getNode(),
   "user-provided value"

javascript/ql/test/query-tests/Security/CWE-089/typed/SqlInjection.expected

@@ -1,41 +1,32 @@
 nodes
-| typedClient.ts:13:7:13:32 | v |
-| typedClient.ts:13:11:13:32 | JSON.pa ... body.x) |
-| typedClient.ts:13:22:13:29 | req.body |
-| typedClient.ts:13:22:13:29 | req.body |
-| typedClient.ts:13:22:13:31 | req.body.x |
-| typedClient.ts:14:24:14:32 | { id: v } |
-| typedClient.ts:14:24:14:32 | { id: v } |
-| typedClient.ts:14:30:14:30 | v |
-| typedClient.ts:21:7:21:32 | v |
-| typedClient.ts:21:11:21:32 | JSON.pa ... body.x) |
-| typedClient.ts:21:22:21:29 | req.body |
-| typedClient.ts:21:22:21:29 | req.body |
-| typedClient.ts:21:22:21:31 | req.body.x |
-| typedClient.ts:22:27:22:35 | { id: v } |
-| typedClient.ts:22:27:22:35 | { id: v } |
-| typedClient.ts:22:33:22:33 | v |
-| typedClient.ts:23:27:23:35 | { id: v } |
-| typedClient.ts:23:27:23:35 | { id: v } |
-| typedClient.ts:23:33:23:33 | v |
+| typedClient.ts:13:7:13:32 | v | semmle.label | v |
+| typedClient.ts:13:11:13:32 | JSON.pa ... body.x) | semmle.label | JSON.pa ... body.x) |
+| typedClient.ts:13:22:13:29 | req.body | semmle.label | req.body |
+| typedClient.ts:13:22:13:31 | req.body.x | semmle.label | req.body.x |
+| typedClient.ts:14:24:14:32 | { id: v } | semmle.label | { id: v } |
+| typedClient.ts:14:30:14:30 | v | semmle.label | v |
+| typedClient.ts:21:7:21:32 | v | semmle.label | v |
+| typedClient.ts:21:11:21:32 | JSON.pa ... body.x) | semmle.label | JSON.pa ... body.x) |
+| typedClient.ts:21:22:21:29 | req.body | semmle.label | req.body |
+| typedClient.ts:21:22:21:31 | req.body.x | semmle.label | req.body.x |
+| typedClient.ts:22:27:22:35 | { id: v } | semmle.label | { id: v } |
+| typedClient.ts:22:33:22:33 | v | semmle.label | v |
+| typedClient.ts:23:27:23:35 | { id: v } | semmle.label | { id: v } |
+| typedClient.ts:23:33:23:33 | v | semmle.label | v |
 edges
 | typedClient.ts:13:7:13:32 | v | typedClient.ts:14:30:14:30 | v |
 | typedClient.ts:13:11:13:32 | JSON.pa ... body.x) | typedClient.ts:13:7:13:32 | v |
 | typedClient.ts:13:22:13:29 | req.body | typedClient.ts:13:22:13:31 | req.body.x |
-| typedClient.ts:13:22:13:29 | req.body | typedClient.ts:13:22:13:31 | req.body.x |
 | typedClient.ts:13:22:13:31 | req.body.x | typedClient.ts:13:11:13:32 | JSON.pa ... body.x) |
 | typedClient.ts:14:30:14:30 | v | typedClient.ts:14:24:14:32 | { id: v } |
-| typedClient.ts:14:30:14:30 | v | typedClient.ts:14:24:14:32 | { id: v } |
 | typedClient.ts:21:7:21:32 | v | typedClient.ts:22:33:22:33 | v |
 | typedClient.ts:21:7:21:32 | v | typedClient.ts:23:33:23:33 | v |
 | typedClient.ts:21:11:21:32 | JSON.pa ... body.x) | typedClient.ts:21:7:21:32 | v |
 | typedClient.ts:21:22:21:29 | req.body | typedClient.ts:21:22:21:31 | req.body.x |
-| typedClient.ts:21:22:21:29 | req.body | typedClient.ts:21:22:21:31 | req.body.x |
 | typedClient.ts:21:22:21:31 | req.body.x | typedClient.ts:21:11:21:32 | JSON.pa ... body.x) |
 | typedClient.ts:22:33:22:33 | v | typedClient.ts:22:27:22:35 | { id: v } |
-| typedClient.ts:22:33:22:33 | v | typedClient.ts:22:27:22:35 | { id: v } |
-| typedClient.ts:23:33:23:33 | v | typedClient.ts:23:27:23:35 | { id: v } |
 | typedClient.ts:23:33:23:33 | v | typedClient.ts:23:27:23:35 | { id: v } |
+subpaths
 #select
 | typedClient.ts:14:24:14:32 | { id: v } | typedClient.ts:13:22:13:29 | req.body | typedClient.ts:14:24:14:32 | { id: v } | This query object depends on a $@. | typedClient.ts:13:22:13:29 | req.body | user-provided value |
 | typedClient.ts:22:27:22:35 | { id: v } | typedClient.ts:21:22:21:29 | req.body | typedClient.ts:22:27:22:35 | { id: v } | This query object depends on a $@. | typedClient.ts:21:22:21:29 | req.body | user-provided value |