Merge pull request #1635 from xiemaisi/js/dont-taint-for-in

Approved by asger-semmle

change-notes/1.22/analysis-javascript.md

@@ -12,6 +12,8 @@
   - [remote-exec](https://www.npmjs.com/package/remote-exec)
 
 * Support for tracking data flow and taint through getter functions (that is, functions that return a property of one of their arguments) and through the receiver object of method calls has been improved. This may produce more security alerts.
+
+* Taint tracking through object property names has been made more precise, resulting in fewer false positive results.
   
 ## New queries
 

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

@@ -231,10 +231,10 @@ module TaintTracking {
       succ.(DataFlow::PropRead).getBase() = pred
       or
       // iterating over a tainted iterator taints the loop variable
-      exists(EnhancedForLoop efl |
-        this = DataFlow::valueNode(efl.getIterationDomain()) and
+      exists(ForOfStmt fos |
+        this = DataFlow::valueNode(fos.getIterationDomain()) and
         pred = this and
-        succ = DataFlow::ssaDefinitionNode(SSA::definition(efl.getIteratorExpr()))
+        succ = DataFlow::ssaDefinitionNode(SSA::definition(fos.getIteratorExpr()))
       )
     }
   }

javascript/ql/test/query-tests/Security/CWE-079/tst.js

@@ -285,3 +285,10 @@ function testCreateContextualFragment() {
     var documentFragment = range.createContextualFragment(tainted); // NOT OK
     document.body.appendChild(documentFragment);
 }
+
+function flowThroughPropertyNames() {
+    var obj = {};
+    obj[Math.random()] = window.name;
+    for (var p in obj)
+      $(p); // OK
+}