JS: Use type-pruning to restrict callback flow

Author: asgerf

javascript/ql/lib/semmle/javascript/dataflow/internal/DataFlowPrivate.qll

@@ -297,16 +297,46 @@ DataFlowCallable nodeGetEnclosingCallable(Node node) {
 }
 
 private newtype TDataFlowType =
-  TTodoDataFlowType() or
-  TTodoDataFlowType2() // Add a dummy value to prevent bad functionality-induced joins arising from a type of size 1.
+  TFunctionType(Function f) or
+  TAnyType()
 
 class DataFlowType extends TDataFlowType {
-  string toString() { result = "" }
+  string toString() {
+    this instanceof TFunctionType and
+    result =
+      "TFunctionType(" + this.asFunction().toString() + ") at line " +
+        this.asFunction().getLocation().getStartLine()
+    or
+    this instanceof TAnyType and result = "TAnyType"
+  }
+
+  Function asFunction() { this = TFunctionType(result) }
 }
 
-predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() }
+/**
+ * Holds if `t1` is strictly stronger than `t2`.
+ */
+predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) {
+  t1 instanceof TFunctionType and t2 = TAnyType()
+}
+
+private DataFlowType getPreciseType(Node node) {
+  exists(Function f |
+    (node = TValueNode(f) or node = TFunctionSelfReferenceNode(f)) and
+    result = TFunctionType(f)
+  )
+  or
+  result = getPreciseType(node.getImmediatePredecessor())
+  or
+  result = getPreciseType(node.(PostUpdateNode).getPreUpdateNode())
+}
 
-DataFlowType getNodeType(Node node) { result = TTodoDataFlowType() and exists(node) }
+DataFlowType getNodeType(Node node) {
+  result = getPreciseType(node)
+  or
+  not exists(getPreciseType(node)) and
+  result = TAnyType()
+}
 
 predicate nodeIsHidden(Node node) {
   DataFlow::PathNode::shouldNodeBeHidden(node)
@@ -341,10 +371,16 @@ predicate neverSkipInPathGraph(Node node) {
   node.asExpr() instanceof VarRef
 }
 
-string ppReprType(DataFlowType t) { none() }
+string ppReprType(DataFlowType t) { result = t.toString() }
 
 pragma[inline]
-predicate compatibleTypes(DataFlowType t1, DataFlowType t2) { any() }
+predicate compatibleTypes(DataFlowType t1, DataFlowType t2) {
+  t1 = t2
+  or
+  t1 instanceof TAnyType and exists(t2)
+  or
+  t2 instanceof TAnyType and exists(t1)
+}
 
 predicate forceHighPrecision(Content c) { none() }
 

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

@@ -5,7 +5,6 @@ legacyDataFlowDifference
 | callbacks.js:37:17:37:24 | source() | callbacks.js:41:10:41:10 | x | only flow with NEW data flow library |
 | callbacks.js:44:17:44:24 | source() | callbacks.js:37:37:37:37 | x | only flow with NEW data flow library |
 | callbacks.js:44:17:44:24 | source() | callbacks.js:38:35:38:35 | x | only flow with NEW data flow library |
-| callbacks.js:73:17:73:24 | source() | callbacks.js:74:35:74:35 | x | only flow with NEW data flow library |
 | capture-flow.js:89:13:89:20 | source() | capture-flow.js:89:6:89:21 | test3c(source()) | only flow with NEW data flow library |
 | capture-flow.js:101:12:101:19 | source() | capture-flow.js:102:6:102:20 | test5("safe")() | only flow with OLD data flow library |
 | capture-flow.js:274:33:274:40 | source() | capture-flow.js:272:10:272:17 | this.foo | only flow with OLD data flow library |
@@ -94,7 +93,6 @@ flow
 | callbacks.js:51:18:51:25 | source() | callbacks.js:30:29:30:29 | y |
 | callbacks.js:53:23:53:30 | source() | callbacks.js:58:10:58:10 | x |
 | callbacks.js:73:17:73:24 | source() | callbacks.js:73:37:73:37 | x |
-| callbacks.js:73:17:73:24 | source() | callbacks.js:74:35:74:35 | x |
 | capture-flow.js:9:11:9:18 | source() | capture-flow.js:14:10:14:16 | outer() |
 | capture-flow.js:9:11:9:18 | source() | capture-flow.js:19:6:19:16 | outerMost() |
 | capture-flow.js:31:14:31:21 | source() | capture-flow.js:31:6:31:22 | confuse(source()) |

javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected

@@ -7,7 +7,6 @@ legacyDataFlowDifference
 | callbacks.js:37:17:37:24 | source() | callbacks.js:41:10:41:10 | x | only flow with NEW data flow library |
 | callbacks.js:44:17:44:24 | source() | callbacks.js:37:37:37:37 | x | only flow with NEW data flow library |
 | callbacks.js:44:17:44:24 | source() | callbacks.js:38:35:38:35 | x | only flow with NEW data flow library |
-| callbacks.js:73:17:73:24 | source() | callbacks.js:74:35:74:35 | x | only flow with NEW data flow library |
 | capture-flow.js:89:13:89:20 | source() | capture-flow.js:89:6:89:21 | test3c(source()) | only flow with NEW data flow library |
 | capture-flow.js:101:12:101:19 | source() | capture-flow.js:102:6:102:20 | test5("safe")() | only flow with OLD data flow library |
 | capture-flow.js:274:33:274:40 | source() | capture-flow.js:272:10:272:17 | this.foo | only flow with OLD data flow library |
@@ -69,7 +68,6 @@ flow
 | callbacks.js:51:18:51:25 | source() | callbacks.js:30:29:30:29 | y |
 | callbacks.js:53:23:53:30 | source() | callbacks.js:58:10:58:10 | x |
 | callbacks.js:73:17:73:24 | source() | callbacks.js:73:37:73:37 | x |
-| callbacks.js:73:17:73:24 | source() | callbacks.js:74:35:74:35 | x |
 | capture-flow.js:9:11:9:18 | source() | capture-flow.js:14:10:14:16 | outer() |
 | capture-flow.js:9:11:9:18 | source() | capture-flow.js:19:6:19:16 | outerMost() |
 | capture-flow.js:31:14:31:21 | source() | capture-flow.js:31:6:31:22 | confuse(source()) |

javascript/ql/test/library-tests/TaintTracking/callbacks.js

@@ -71,5 +71,5 @@ function forwardTaint4(x, cb) {
 
 function test2() {
   forwardTaint4(source(), x => sink(x)); // NOT OK
-  forwardTaint4("safe", x => sink(x)); // OK [INCONSISTENCY]
+  forwardTaint4("safe", x => sink(x)); // OK
 }