Merge pull request #1351 from asger-semmle/js-incomplete-nodes

JS: Mark some more nodes as incomplete

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

@@ -1270,6 +1270,16 @@ module DataFlow {
       (e instanceof AwaitExpr or e instanceof DynamicImportExpr) and
       cause = "await"
     )
+    or
+    nd instanceof TExceptionalInvocationReturnNode and cause = "call"
+    or
+    nd instanceof TExceptionalFunctionReturnNode and cause = "call"
+    or
+    exists(PropertyPattern p | nd = TPropNode(p)) and cause = "heap"
+    or
+    nd instanceof TElementPatternNode and cause = "heap"
+    or
+    nd instanceof UnusedParameterNode and cause = "call"
   }
 
   /**
@@ -1288,9 +1298,6 @@ module DataFlow {
     or
     exists(ComprehensionBlock cb | def = cb.getIterator()) and
     cause = "yield"
-    or
-    def.getTarget() instanceof DestructuringPattern and
-    cause = "heap"
   }
   import Nodes
   import Sources

javascript/ql/test/library-tests/DataFlow/incomplete.expected

@@ -1,21 +1,39 @@
+| eval.js:1:1:5:1 | exceptional return of function k | call |
 | eval.js:2:7:2:12 | x | eval |
 | eval.js:3:3:3:6 | eval | global |
 | eval.js:3:3:3:16 | eval("x = 23") | call |
+| eval.js:3:3:3:16 | exceptional return of eval("x = 23") | call |
+| sources.js:1:1:1:12 | exceptional return of new (x => x) | call |
 | sources.js:1:6:1:6 | x | call |
+| sources.js:1:6:1:11 | exceptional return of anonymous function | call |
+| sources.js:3:1:5:6 | exceptional return of (functi ... \\n})(23) | call |
+| sources.js:3:2:5:1 | exceptional return of anonymous function | call |
 | tst.js:1:10:1:11 | fs | import |
+| tst.js:16:1:20:9 | exceptional return of (functi ... ("arg") | call |
+| tst.js:16:2:20:1 | exceptional return of function f | call |
 | tst.js:17:7:17:10 | Math | global |
 | tst.js:17:7:17:17 | Math.random | global |
 | tst.js:17:7:17:17 | Math.random | heap |
 | tst.js:17:7:17:19 | Math.random() | call |
-| tst.js:22:5:22:25 | readFileSync | heap |
+| tst.js:17:7:17:19 | exceptional return of Math.random() | call |
+| tst.js:22:7:22:18 | readFileSync | heap |
+| tst.js:28:1:30:3 | exceptional return of (() =>\\n ... les\\n)() | call |
+| tst.js:28:2:29:3 | exceptional return of anonymous function | call |
+| tst.js:32:1:34:1 | exceptional return of function g | call |
+| tst.js:32:12:32:12 | b | call |
+| tst.js:35:1:35:7 | exceptional return of g(true) | call |
 | tst.js:35:1:35:7 | g(true) | call |
+| tst.js:39:4:41:3 | exceptional return of method m | call |
 | tst.js:40:5:40:8 | this | call |
 | tst.js:43:1:43:3 | o.x | heap |
 | tst.js:44:1:44:3 | o.m | heap |
+| tst.js:44:1:44:5 | exceptional return of o.m() | call |
 | tst.js:44:1:44:5 | o.m() | call |
 | tst.js:47:1:47:6 | global | global |
 | tst.js:49:17:49:17 | B | global |
+| tst.js:50:14:53:3 | exceptional return of constructor of class A | call |
 | tst.js:51:5:51:9 | super | call |
+| tst.js:51:5:51:13 | exceptional return of super(42) | call |
 | tst.js:51:5:51:13 | super(42) | call |
 | tst.js:52:5:52:14 | new.target | call |
 | tst.js:58:1:58:3 | tag | global |
@@ -23,44 +41,56 @@
 | tst.js:61:1:61:5 | ::o.m | call |
 | tst.js:61:3:61:5 | o.m | heap |
 | tst.js:62:1:62:4 | o::g | call |
+| tst.js:64:1:67:1 | exceptional return of function h | call |
 | tst.js:65:3:65:10 | yield 42 | yield |
 | tst.js:66:13:66:25 | function.sent | yield |
+| tst.js:68:12:68:14 | exceptional return of h() | call |
 | tst.js:68:12:68:14 | h() | call |
 | tst.js:69:1:69:9 | iter.next | heap |
+| tst.js:69:1:69:13 | exceptional return of iter.next(23) | call |
 | tst.js:69:1:69:13 | iter.next(23) | call |
+| tst.js:71:1:73:1 | exceptional return of function k | call |
 | tst.js:72:3:72:11 | await p() | await |
 | tst.js:72:9:72:9 | p | global |
+| tst.js:72:9:72:11 | exceptional return of p() | call |
 | tst.js:72:9:72:11 | p() | call |
 | tst.js:75:9:75:21 | import('foo') | await |
 | tst.js:77:10:77:10 | i | heap |
 | tst.js:80:10:80:10 | v | heap |
 | tst.js:83:18:83:18 | v | yield |
 | tst.js:85:18:85:18 | v | yield |
-| tst.js:87:11:87:24 | o | heap |
+| tst.js:87:1:96:2 | exceptional return of (functi ... r: 0\\n}) | call |
-| tst.js:87:11:87:24 | x | heap |
+| tst.js:87:2:92:1 | exceptional return of anonymous function | call |
-| tst.js:88:7:88:18 | y | heap |
+| tst.js:87:13:87:16 | p: x | heap |
-| tst.js:90:4:90:15 | z | heap |
+| tst.js:88:9:88:12 | q: y | heap |
-| tst.js:98:11:98:24 | rest | heap |
+| tst.js:90:6:90:9 | r: z | heap |
-| tst.js:98:11:98:24 | x | heap |
+| tst.js:98:1:103:17 | exceptional return of (functi ... 3, 0 ]) | call |
-| tst.js:99:7:99:18 | y | heap |
+| tst.js:98:2:103:1 | exceptional return of anonymous function | call |
-| tst.js:101:3:101:16 | z | heap |
+| tst.js:98:13:98:13 | x | heap |
-| tst.js:108:6:108:38 | v1a | heap |
+| tst.js:99:9:99:9 | y | heap |
-| tst.js:108:6:108:38 | v1b | heap |
+| tst.js:101:7:101:7 | z | heap |
-| tst.js:108:6:108:38 | v1c | heap |
+| tst.js:107:2:113:1 | exceptional return of anonymous function | call |
+| tst.js:108:7:108:9 | v1a | heap |
+| tst.js:108:12:108:20 | v1b = o1b | heap |
 | tst.js:108:18:108:20 | o1b | global |
+| tst.js:108:23:108:31 | v1c = o1c | heap |
 | tst.js:108:29:108:31 | o1c | global |
 | tst.js:108:36:108:38 | o1d | global |
-| tst.js:111:6:111:38 | v2a | heap |
+| tst.js:111:7:111:9 | v2a | heap |
-| tst.js:111:6:111:38 | v2b | heap |
+| tst.js:111:12:111:14 | v2b | heap |
-| tst.js:111:6:111:38 | v2c | heap |
 | tst.js:111:18:111:20 | o2b | global |
+| tst.js:111:23:111:25 | v2c | heap |
 | tst.js:111:29:111:31 | o2c | global |
 | tst.js:111:36:111:38 | o2d | global |
 | tst.ts:2:14:2:19 | x | namespace |
+| tst.ts:3:3:3:8 | exceptional return of setX() | call |
 | tst.ts:3:3:3:8 | setX() | call |
+| tst.ts:7:1:9:1 | exceptional return of function setX | call |
 | tst.ts:8:3:8:5 | A.x | heap |
 | tst.ts:11:11:11:13 | A.x | heap |
 | tst.ts:13:26:13:29 | List | global |
 | tst.ts:13:39:13:38 | args | call |
+| tst.ts:13:39:13:38 | exceptional return of default constructor of class StringList | call |
+| tst.ts:13:39:13:38 | exceptional return of super(...args) | call |
 | tst.ts:13:39:13:38 | super | call |
 | tst.ts:13:39:13:38 | super(...args) | call |

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

@@ -1,3 +1,9 @@
+typeInferenceMismatch
+| addexpr.js:4:10:4:17 | source() | addexpr.js:4:5:4:17 | x |
+| addexpr.js:4:10:4:17 | source() | addexpr.js:6:3:6:14 | x |
+| addexpr.js:11:15:11:22 | source() | addexpr.js:17:5:17:18 | value |
+| addexpr.js:11:15:11:22 | source() | addexpr.js:19:3:19:14 | value |
+#select
 | access-path-sanitizer.js:2:18:2:25 | source() | access-path-sanitizer.js:4:8:4:12 | obj.x |
 | addexpr.js:4:10:4:17 | source() | addexpr.js:7:8:7:8 | x |
 | addexpr.js:11:15:11:22 | source() | addexpr.js:21:8:21:12 | value |

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

@@ -1,13 +1,33 @@
 import javascript
+import semmle.javascript.dataflow.InferredTypes
 
 DataFlow::CallNode getACall(string name) { result.getCalleeName() = name }
 
+class Sink extends DataFlow::Node {
+  Sink() { this = getACall("sink").getAnArgument() }
+}
+
+/**
+ * A node that shouldn't be taintable according to the type inference,
+ * as it claims to be neither an object nor a string.
+ */
+class UntaintableNode extends DataFlow::Node {
+  UntaintableNode() {
+    not analyze().getAType() = TTObject() and
+    not analyze().getAType() = TTString()
+  }
+}
+
 class BasicConfig extends TaintTracking::Configuration {
   BasicConfig() { this = "BasicConfig" }
 
   override predicate isSource(DataFlow::Node node) { node = getACall("source") }
 
-  override predicate isSink(DataFlow::Node node) { node = getACall("sink").getAnArgument() }
+  override predicate isSink(DataFlow::Node node) {
+    node instanceof Sink
+    or
+    node instanceof UntaintableNode
+  }
 
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode node) {
     node instanceof BasicSanitizerGuard
@@ -22,6 +42,10 @@ class BasicSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::C
   }
 }
 
-from BasicConfig cfg, DataFlow::Node src, DataFlow::Node sink
+query predicate typeInferenceMismatch(DataFlow::Node source, UntaintableNode sink) {
+  any(BasicConfig cfg).hasFlow(source, sink)
+}
+
+from BasicConfig cfg, DataFlow::Node src, Sink sink
 where cfg.hasFlow(src, sink)
 select src, sink