JS: Add taint step through destructuring for-of loop

Author: asger-semmle

javascript/ql/src/semmle/javascript/Stmt.qll

@@ -818,6 +818,15 @@ abstract class EnhancedForLoop extends LoopStmt {
     result = getIterator().(DeclStmt).getADecl()
   }
 
+  /**
+   * Gets the property, variable, or destructuring pattern occurring as the iterator
+   * expression in this `for`-`in` or `for`-`of` loop.
+   */
+  Expr getLValue() {
+    result = getIteratorExpr() or
+    result = getIterator().(DeclStmt).getADecl().getBindingPattern()
+  }
+
   /**
    * Gets an iterator variable of this `for`-`in` or `for`-`of` loop.
    */

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

@@ -232,7 +232,7 @@ module TaintTracking {
       exists(ForOfStmt fos |
         this = DataFlow::valueNode(fos.getIterationDomain()) and
         pred = this and
-        succ = DataFlow::ssaDefinitionNode(SSA::definition(fos.getIteratorExpr()))
+        succ = DataFlow::lvalueNode(fos.getLValue())
       )
     }
   }

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

@@ -3,6 +3,7 @@ typeInferenceMismatch
 | 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 |
+| destruct.js:20:7:20:14 | source() | destruct.js:13:14:13:19 | [a, b] |
 #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 |
@@ -38,9 +39,11 @@ typeInferenceMismatch
 | constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:30:8:30:19 | d_safe.taint |
 | constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
 | constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
-| destruct.js:15:7:15:14 | source() | destruct.js:5:10:5:10 | z |
-| destruct.js:15:7:15:14 | source() | destruct.js:8:10:8:10 | w |
-| destruct.js:15:7:15:14 | source() | destruct.js:11:10:11:10 | q |
+| destruct.js:20:7:20:14 | source() | destruct.js:5:10:5:10 | z |
+| destruct.js:20:7:20:14 | source() | destruct.js:8:10:8:10 | w |
+| destruct.js:20:7:20:14 | source() | destruct.js:11:10:11:10 | q |
+| destruct.js:20:7:20:14 | source() | destruct.js:14:12:14:12 | a |
+| destruct.js:20:7:20:14 | source() | destruct.js:15:12:15:12 | b |
 | exceptions.js:3:15:3:22 | source() | exceptions.js:5:10:5:10 | e |
 | exceptions.js:21:17:21:24 | source() | exceptions.js:23:10:23:10 | e |
 | exceptions.js:21:17:21:24 | source() | exceptions.js:24:10:24:21 | e.toString() |

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

@@ -9,6 +9,11 @@ function test() {
 
     let { x: [ { y: q } ] } = obj;
     sink(q); // NOT OK
+
+    for (let [a, b] of obj) {
+      sink(a); // NOT OK
+      sink(b); // NOT OK
+    }
   }
 
   function g() {