python: rewrite NoSQLInjection to use flow state

This allows a bit more precision. Specifically, we could
 require the sanitizer to only affect `ConvertedToDict`.
 In practice, most sanitizers woudl probably fail on raw
 input also, though.

Author: yoff

python/ql/src/experimental/Security/CWE-943/NoSQLInjection.ql

@@ -12,7 +12,7 @@
 import python
 import experimental.semmle.python.security.injection.NoSQLInjection
 
-from CustomPathNode source, CustomPathNode sink
-where noSQLInjectionFlow(source, sink)
+from NoSQLInjection::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
+where config.hasFlowPath(source, sink)
 select sink, source, sink, "$@ NoSQL query contains an unsanitized $@", sink, "This", source,
   "user-provided value"

python/ql/src/experimental/semmle/python/security/injection/NoSQLInjection.qll

@@ -1,57 +1,54 @@
 import python
 import semmle.python.dataflow.new.DataFlow
-import semmle.python.dataflow.new.DataFlow2
 import semmle.python.dataflow.new.TaintTracking
-import semmle.python.dataflow.new.TaintTracking2
 import semmle.python.dataflow.new.RemoteFlowSources
-import semmle.python.security.dataflow.ChainedConfigs12
 import experimental.semmle.python.Concepts
 import semmle.python.Concepts
 
-/**
- * A taint-tracking configuration for detecting string-to-dict conversions.
- */
-class RFSToDictConfig extends TaintTracking::Configuration {
-  RFSToDictConfig() { this = "RFSToDictConfig" }
-
-  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
-
-  override predicate isSink(DataFlow::Node sink) {
-    exists(Decoding decoding | decoding.getFormat() = "JSON" and sink = decoding.getOutput())
+module NoSQLInjection {
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "NoSQLInjection" }
+
+    override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
+      source instanceof RemoteFlowSource and
+      state instanceof RemoteInput
+    }
+
+    override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
+      sink = any(NoSQLQuery noSQLQuery).getQuery() and
+      state instanceof ConvertedToDict
+    }
+
+    override predicate isBarrier(DataFlow::Node node, DataFlow::FlowState state) {
+      // Block `RemoteInput` paths here, since they change state to `ConvertedToDict`
+      exists(Decoding decoding | decoding.getFormat() = "JSON" and node = decoding.getOutput()) and
+      state instanceof RemoteInput
+    }
+
+    override predicate isAdditionalFlowStep(
+      DataFlow::Node nodeFrom, DataFlow::FlowState stateFrom, DataFlow::Node nodeTo,
+      DataFlow::FlowState stateTo
+    ) {
+      exists(Decoding decoding | decoding.getFormat() = "JSON" |
+        nodeFrom = decoding.getAnInput() and
+        nodeTo = decoding.getOutput()
+      ) and
+      stateFrom instanceof RemoteInput and
+      stateTo instanceof ConvertedToDict
+    }
+
+    override predicate isSanitizer(DataFlow::Node sanitizer) {
+      sanitizer = any(NoSQLSanitizer noSQLSanitizer).getAnInput()
+    }
   }
 
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer = any(NoSQLSanitizer noSQLSanitizer).getAnInput()
+  /** A flow state signifying remote input. */
+  class RemoteInput extends DataFlow::FlowState {
+    RemoteInput() { this = "RemoteInput" }
   }
-}
 
-/**
- * A taint-tracking configuration for detecting NoSQL injections (previously converted to a dict).
- */
-class FromDataDictToSink extends TaintTracking2::Configuration {
-  FromDataDictToSink() { this = "FromDataDictToSink" }
-
-  override predicate isSource(DataFlow::Node source) {
-    exists(Decoding decoding | decoding.getFormat() = "JSON" and source = decoding.getOutput())
+  /** A flow state signifying remote input converted to a dictionary. */
+  class ConvertedToDict extends DataFlow::FlowState {
+    ConvertedToDict() { this = "ConvertedToDict" }
   }
-
-  override predicate isSink(DataFlow::Node sink) { sink = any(NoSQLQuery noSQLQuery).getQuery() }
-
-  override predicate isSanitizer(DataFlow::Node sanitizer) {
-    sanitizer = any(NoSQLSanitizer noSQLSanitizer).getAnInput()
-  }
-}
-
-/**
- * A predicate checking string-to-dict conversion and its arrival to a NoSQL injection sink.
- */
-predicate noSQLInjectionFlow(CustomPathNode source, CustomPathNode sink) {
-  exists(
-    RFSToDictConfig config, DataFlow::PathNode mid1, DataFlow2::PathNode mid2,
-    FromDataDictToSink config2
-  |
-    config.hasFlowPath(source.asNode1(), mid1) and
-    config2.hasFlowPath(mid2, sink.asNode2()) and
-    mid1.getNode().asCfgNode() = mid2.getNode().asCfgNode()
-  )
 }