From 3b28bdbeeddaedbe0a0ff30a819e224aa212e8c3 Mon Sep 17 00:00:00 2001
From: Asger Feldthaus <asgerf@github.com>
Date: Wed, 5 Feb 2020 15:28:42 +0000
Subject: [PATCH] JS: Rewrite AnalyzedThisInArrayIterationFunction

---
 .../src/semmle/javascript/StandardLibrary.qll | 29 +++++++------------
 .../internal/InterProceduralTypeInference.qll | 21 ++++++++++++++
 2 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/javascript/ql/src/semmle/javascript/StandardLibrary.qll b/javascript/ql/src/semmle/javascript/StandardLibrary.qll
index 0de4cce8e05..aae98553faf 100644
--- a/javascript/ql/src/semmle/javascript/StandardLibrary.qll
+++ b/javascript/ql/src/semmle/javascript/StandardLibrary.qll
@@ -46,33 +46,26 @@ class DirectEval extends CallExpr {
 }
 
 /**
- * Flow analysis for `this` expressions inside a function that is called with
- * `Array.prototype.map` or a similar Array function that binds `this`.
- *
- * However, since the function could be invoked in another way, we additionally
- * still infer the ordinary abstract value.
+ * Models `Array.prototype.map` and friends as partial invocations that pass their second
+ * argument as the receiver to the callback.
  */
-private class AnalyzedThisInArrayIterationFunction extends AnalyzedNode, DataFlow::ThisNode {
-  AnalyzedNode thisSource;
-
-  AnalyzedThisInArrayIterationFunction() {
-    exists(DataFlow::MethodCallNode bindingCall, string name |
+private class ArrayIterationCallbackAsPartialInvoke extends DataFlow::PartialInvokeNode::Range, DataFlow::MethodCallNode {
+  ArrayIterationCallbackAsPartialInvoke() {
+    getNumArgument() = 2 and
+    // Filter out library methods named 'forEach' etc
+    not DataFlow::moduleImport(_).flowsTo(getReceiver()) and
+    exists(string name | name = getMethodName() |
       name = "filter" or
       name = "forEach" or
       name = "map" or
       name = "some" or
       name = "every"
-    |
-      name = bindingCall.getMethodName() and
-      2 = bindingCall.getNumArgument() and
-      getBinder() = bindingCall.getCallback(0) and
-      thisSource = bindingCall.getArgument(1)
     )
   }
 
-  override AbstractValue getALocalValue() {
-    result = thisSource.getALocalValue() or
-    result = AnalyzedNode.super.getALocalValue()
+  override DataFlow::Node getBoundReceiver(DataFlow::Node callback) {
+    callback = getArgument(0) and
+    result = getArgument(1)
   }
 }
 
diff --git a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
index d2243aeff7a..ea46bf36668 100644
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterProceduralTypeInference.qll
@@ -274,3 +274,24 @@ private class TypeInferredMethodWithAnalyzedReturnFlow extends CallWithNonLocalA
 
   override AnalyzedFunction getACallee() { result = fun }
 }
+
+/**
+ * Propagates receivers into locally defined callbacks of partial invocations.
+ */
+private class AnalyzedThisInPartialInvokeCallback extends AnalyzedNode, DataFlow::ThisNode {
+  DataFlow::PartialInvokeNode call;
+  DataFlow::Node receiver;
+
+  AnalyzedThisInPartialInvokeCallback() {
+    exists(DataFlow::Node callbackArg |
+      receiver = call.getBoundReceiver(callbackArg) and
+      getBinder().flowsTo(callbackArg)
+    )
+  }
+
+  override AbstractValue getALocalValue() {
+    result = receiver.analyze().getALocalValue()
+    or
+    result = AnalyzedNode.super.getALocalValue()
+  }
+}