JS: Port TypeConfusionThroughParameterTampering

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

@@ -23,6 +23,21 @@ module TypeConfusionThroughParameterTampering {
    */
   abstract class Barrier extends DataFlow::Node { }
 
+  /**
+   * A barrier guard for type confusion for HTTP request inputs.
+   */
+  abstract class BarrierGuard extends DataFlow::Node {
+    /**
+     * Holds if this node acts as a barrier for data flow, blocking further flow from `e` if `this` evaluates to `outcome`.
+     */
+    predicate blocksExpr(boolean outcome, Expr e) { none() }
+  }
+
+  /** A subclass of `BarrierGuard` that is used for backward compatibility with the old data flow library. */
+  abstract class BarrierGuardLegacy extends BarrierGuard, TaintTracking::SanitizerGuardNode {
+    override predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) }
+  }
+
   /**
    * An HTTP request parameter that the user controls the type of.
    *

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

@@ -13,19 +13,61 @@ private import semmle.javascript.dataflow.InferredTypes
 import TypeConfusionThroughParameterTamperingCustomizations::TypeConfusionThroughParameterTampering
 
 /**
- * A taint tracking configuration for type confusion for HTTP request inputs.
+ * Data flow configuration for type confusion for HTTP request inputs.
  */
-class Configuration extends DataFlow::Configuration {
+module TypeConfusionConfig implements DataFlow::ConfigSig {
-  Configuration() { this = "TypeConfusionThroughParameterTampering" }
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+  predicate isSink(DataFlow::Node sink) {
-
-  override predicate isSink(DataFlow::Node sink) {
     sink instanceof Sink and
     sink.analyze().getAType() = TTString() and
     sink.analyze().getAType() = TTObject()
   }
 
+  predicate isBarrier(DataFlow::Node node) {
+    node instanceof Barrier or node = DataFlow::MakeBarrierGuard<BarrierGuard>::getABarrierNode()
+  }
+}
+
+/**
+ * Data flow for type confusion for HTTP request inputs.
+ */
+module TypeConfusionFlow = DataFlow::Global<TypeConfusionConfig>;
+
+private class TypeOfTestBarrier extends BarrierGuardLegacy, DataFlow::ValueNode {
+  override EqualityTest astNode;
+
+  TypeOfTestBarrier() { TaintTracking::isTypeofGuard(astNode, _, _) }
+
+  override predicate blocksExpr(boolean outcome, Expr e) {
+    exists(string tag |
+      TaintTracking::isTypeofGuard(astNode, e, tag) and
+      if tag = ["string", "object"]
+      then outcome = [true, false] // separation between string/array removes type confusion in both branches
+      else outcome = astNode.getPolarity() // block flow to branch where value is neither string nor array
+    )
+  }
+}
+
+private class IsArrayBarrier extends BarrierGuardLegacy, DataFlow::CallNode {
+  IsArrayBarrier() { this = DataFlow::globalVarRef("Array").getAMemberCall("isArray") }
+
+  override predicate blocksExpr(boolean outcome, Expr e) {
+    e = this.getArgument(0).asExpr() and
+    outcome = [true, false] // separation between string/array removes type confusion in both branches
+  }
+}
+
+/**
+ * DEPRECATED. Use the `TypeConfusionFlow` module instead.
+ */
+deprecated class Configuration extends DataFlow::Configuration {
+  Configuration() { this = "TypeConfusionThroughParameterTampering" }
+
+  override predicate isSource(DataFlow::Node source) { TypeConfusionConfig::isSource(source) }
+
+  override predicate isSink(DataFlow::Node sink) { TypeConfusionConfig::isSink(sink) }
+
   override predicate isBarrier(DataFlow::Node node) {
     super.isBarrier(node)
     or
@@ -37,27 +79,3 @@ class Configuration extends DataFlow::Configuration {
     guard instanceof IsArrayBarrier
   }
 }
-
-private class TypeOfTestBarrier extends DataFlow::BarrierGuardNode, DataFlow::ValueNode {
-  override EqualityTest astNode;
-
-  TypeOfTestBarrier() { TaintTracking::isTypeofGuard(astNode, _, _) }
-
-  override predicate blocks(boolean outcome, Expr e) {
-    exists(string tag |
-      TaintTracking::isTypeofGuard(astNode, e, tag) and
-      if tag = ["string", "object"]
-      then outcome = [true, false] // separation between string/array removes type confusion in both branches
-      else outcome = astNode.getPolarity() // block flow to branch where value is neither string nor array
-    )
-  }
-}
-
-private class IsArrayBarrier extends DataFlow::BarrierGuardNode, DataFlow::CallNode {
-  IsArrayBarrier() { this = DataFlow::globalVarRef("Array").getAMemberCall("isArray") }
-
-  override predicate blocks(boolean outcome, Expr e) {
-    e = this.getArgument(0).asExpr() and
-    outcome = [true, false] // separation between string/array removes type confusion in both branches
-  }
-}

javascript/ql/src/Security/CWE-843/TypeConfusionThroughParameterTampering.ql

@@ -12,10 +12,10 @@
 
 import javascript
 import semmle.javascript.security.dataflow.TypeConfusionThroughParameterTamperingQuery
-import DataFlow::PathGraph
+import TypeConfusionFlow::PathGraph
 
-from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+from TypeConfusionFlow::PathNode source, TypeConfusionFlow::PathNode sink
-where cfg.hasFlowPath(source, sink)
+where TypeConfusionFlow::flowPath(source, sink)
 select sink.getNode(), source, sink,
   "Potential type confusion as $@ may be either an array or a string.", source.getNode(),
   "this HTTP request parameter"

javascript/ql/test/query-tests/Security/CWE-843/TypeConfusionThroughParameterTampering.expected

@@ -1,81 +1,76 @@
-nodes
-| tst.js:5:9:5:27 | foo |
-| tst.js:5:15:5:27 | req.query.foo |
-| tst.js:5:15:5:27 | req.query.foo |
-| tst.js:6:5:6:7 | foo |
-| tst.js:6:5:6:7 | foo |
-| tst.js:8:5:8:7 | foo |
-| tst.js:8:5:8:7 | foo |
-| tst.js:11:9:11:11 | foo |
-| tst.js:11:9:11:11 | foo |
-| tst.js:14:16:14:18 | bar |
-| tst.js:15:9:15:11 | bar |
-| tst.js:15:9:15:11 | bar |
-| tst.js:17:7:17:9 | foo |
-| tst.js:27:5:27:7 | foo |
-| tst.js:27:5:27:7 | foo |
-| tst.js:28:5:28:7 | foo |
-| tst.js:28:5:28:7 | foo |
-| tst.js:45:9:45:35 | foo |
-| tst.js:45:15:45:35 | ctx.req ... ery.foo |
-| tst.js:45:15:45:35 | ctx.req ... ery.foo |
-| tst.js:46:5:46:7 | foo |
-| tst.js:46:5:46:7 | foo |
-| tst.js:77:25:77:38 | req.query.path |
-| tst.js:77:25:77:38 | req.query.path |
-| tst.js:80:23:80:23 | p |
-| tst.js:81:9:81:9 | p |
-| tst.js:81:9:81:9 | p |
-| tst.js:82:9:82:9 | p |
-| tst.js:82:9:82:9 | p |
-| tst.js:90:5:90:12 | data.foo |
-| tst.js:90:5:90:12 | data.foo |
-| tst.js:90:5:90:12 | data.foo |
-| tst.js:92:9:92:16 | data.foo |
-| tst.js:92:9:92:16 | data.foo |
-| tst.js:92:9:92:16 | data.foo |
-| tst.js:98:9:98:16 | data.foo |
-| tst.js:98:9:98:16 | data.foo |
-| tst.js:98:9:98:16 | data.foo |
-| tst.js:103:9:103:29 | data |
-| tst.js:103:16:103:29 | req.query.data |
-| tst.js:103:16:103:29 | req.query.data |
-| tst.js:104:5:104:8 | data |
-| tst.js:104:5:104:8 | data |
 edges
 | tst.js:5:9:5:27 | foo | tst.js:6:5:6:7 | foo |
 | tst.js:5:9:5:27 | foo | tst.js:6:5:6:7 | foo |
 | tst.js:5:9:5:27 | foo | tst.js:8:5:8:7 | foo |
 | tst.js:5:9:5:27 | foo | tst.js:8:5:8:7 | foo |
-| tst.js:5:9:5:27 | foo | tst.js:11:9:11:11 | foo |
-| tst.js:5:9:5:27 | foo | tst.js:11:9:11:11 | foo |
 | tst.js:5:9:5:27 | foo | tst.js:17:7:17:9 | foo |
+| tst.js:5:9:5:27 | foo | tst.js:21:5:21:7 | foo |
+| tst.js:5:9:5:27 | foo | tst.js:22:5:22:7 | foo |
+| tst.js:5:9:5:27 | foo | tst.js:23:5:23:7 | foo |
+| tst.js:5:9:5:27 | foo | tst.js:25:5:25:7 | foo |
 | tst.js:5:9:5:27 | foo | tst.js:27:5:27:7 | foo |
 | tst.js:5:9:5:27 | foo | tst.js:27:5:27:7 | foo |
 | tst.js:5:9:5:27 | foo | tst.js:28:5:28:7 | foo |
-| tst.js:5:9:5:27 | foo | tst.js:28:5:28:7 | foo |
 | tst.js:5:15:5:27 | req.query.foo | tst.js:5:9:5:27 | foo |
-| tst.js:5:15:5:27 | req.query.foo | tst.js:5:9:5:27 | foo |
+| tst.js:6:5:6:7 | foo | tst.js:8:5:8:7 | foo |
-| tst.js:14:16:14:18 | bar | tst.js:15:9:15:11 | bar |
+| tst.js:6:5:6:7 | foo | tst.js:8:5:8:7 | foo |
+| tst.js:8:5:8:7 | foo | tst.js:10:5:12:5 | functio ... K\\n    } [foo] |
+| tst.js:8:5:8:7 | foo | tst.js:17:7:17:9 | foo |
+| tst.js:10:5:12:5 | functio ... K\\n    } [foo] | tst.js:10:14:10:14 | f [foo] |
+| tst.js:10:5:12:5 | functio ... K\\n    } [foo] | tst.js:11:9:11:11 | foo |
+| tst.js:10:14:10:14 | f [foo] | tst.js:39:12:39:12 | f [foo] |
 | tst.js:14:16:14:18 | bar | tst.js:15:9:15:11 | bar |
 | tst.js:17:7:17:9 | foo | tst.js:14:16:14:18 | bar |
-| tst.js:45:9:45:35 | foo | tst.js:46:5:46:7 | foo |
+| tst.js:17:7:17:9 | foo | tst.js:21:5:21:7 | foo |
+| tst.js:21:5:21:7 | foo | tst.js:22:5:22:7 | foo |
+| tst.js:22:5:22:7 | foo | tst.js:23:5:23:7 | foo |
+| tst.js:23:5:23:7 | foo | tst.js:25:5:25:7 | foo |
+| tst.js:25:5:25:7 | foo | tst.js:27:5:27:7 | foo |
+| tst.js:25:5:25:7 | foo | tst.js:27:5:27:7 | foo |
+| tst.js:27:5:27:7 | foo | tst.js:28:5:28:7 | foo |
+| tst.js:39:12:39:12 | f [foo] | tst.js:11:9:11:11 | foo |
 | tst.js:45:9:45:35 | foo | tst.js:46:5:46:7 | foo |
 | tst.js:45:15:45:35 | ctx.req ... ery.foo | tst.js:45:9:45:35 | foo |
-| tst.js:45:15:45:35 | ctx.req ... ery.foo | tst.js:45:9:45:35 | foo |
-| tst.js:77:25:77:38 | req.query.path | tst.js:80:23:80:23 | p |
 | tst.js:77:25:77:38 | req.query.path | tst.js:80:23:80:23 | p |
 | tst.js:80:23:80:23 | p | tst.js:81:9:81:9 | p |
-| tst.js:80:23:80:23 | p | tst.js:81:9:81:9 | p |
 | tst.js:80:23:80:23 | p | tst.js:82:9:82:9 | p |
-| tst.js:80:23:80:23 | p | tst.js:82:9:82:9 | p |
-| tst.js:90:5:90:12 | data.foo | tst.js:90:5:90:12 | data.foo |
-| tst.js:92:9:92:16 | data.foo | tst.js:92:9:92:16 | data.foo |
-| tst.js:98:9:98:16 | data.foo | tst.js:98:9:98:16 | data.foo |
-| tst.js:103:9:103:29 | data | tst.js:104:5:104:8 | data |
 | tst.js:103:9:103:29 | data | tst.js:104:5:104:8 | data |
 | tst.js:103:16:103:29 | req.query.data | tst.js:103:9:103:29 | data |
-| tst.js:103:16:103:29 | req.query.data | tst.js:103:9:103:29 | data |
+nodes
+| tst.js:5:9:5:27 | foo | semmle.label | foo |
+| tst.js:5:15:5:27 | req.query.foo | semmle.label | req.query.foo |
+| tst.js:6:5:6:7 | foo | semmle.label | foo |
+| tst.js:6:5:6:7 | foo | semmle.label | foo |
+| tst.js:8:5:8:7 | foo | semmle.label | foo |
+| tst.js:8:5:8:7 | foo | semmle.label | foo |
+| tst.js:10:5:12:5 | functio ... K\\n    } [foo] | semmle.label | functio ... K\\n    } [foo] |
+| tst.js:10:14:10:14 | f [foo] | semmle.label | f [foo] |
+| tst.js:11:9:11:11 | foo | semmle.label | foo |
+| tst.js:14:16:14:18 | bar | semmle.label | bar |
+| tst.js:15:9:15:11 | bar | semmle.label | bar |
+| tst.js:17:7:17:9 | foo | semmle.label | foo |
+| tst.js:21:5:21:7 | foo | semmle.label | foo |
+| tst.js:22:5:22:7 | foo | semmle.label | foo |
+| tst.js:23:5:23:7 | foo | semmle.label | foo |
+| tst.js:25:5:25:7 | foo | semmle.label | foo |
+| tst.js:27:5:27:7 | foo | semmle.label | foo |
+| tst.js:27:5:27:7 | foo | semmle.label | foo |
+| tst.js:28:5:28:7 | foo | semmle.label | foo |
+| tst.js:39:12:39:12 | f [foo] | semmle.label | f [foo] |
+| tst.js:45:9:45:35 | foo | semmle.label | foo |
+| tst.js:45:15:45:35 | ctx.req ... ery.foo | semmle.label | ctx.req ... ery.foo |
+| tst.js:46:5:46:7 | foo | semmle.label | foo |
+| tst.js:77:25:77:38 | req.query.path | semmle.label | req.query.path |
+| tst.js:80:23:80:23 | p | semmle.label | p |
+| tst.js:81:9:81:9 | p | semmle.label | p |
+| tst.js:82:9:82:9 | p | semmle.label | p |
+| tst.js:90:5:90:12 | data.foo | semmle.label | data.foo |
+| tst.js:92:9:92:16 | data.foo | semmle.label | data.foo |
+| tst.js:98:9:98:16 | data.foo | semmle.label | data.foo |
+| tst.js:103:9:103:29 | data | semmle.label | data |
+| tst.js:103:16:103:29 | req.query.data | semmle.label | req.query.data |
+| tst.js:104:5:104:8 | data | semmle.label | data |
+subpaths
 #select
 | tst.js:6:5:6:7 | foo | tst.js:5:15:5:27 | req.query.foo | tst.js:6:5:6:7 | foo | Potential type confusion as $@ may be either an array or a string. | tst.js:5:15:5:27 | req.query.foo | this HTTP request parameter |
 | tst.js:8:5:8:7 | foo | tst.js:5:15:5:27 | req.query.foo | tst.js:8:5:8:7 | foo | Potential type confusion as $@ may be either an array or a string. | tst.js:5:15:5:27 | req.query.foo | this HTTP request parameter |