JavaScript: Teach type trackers to track flow through one level of properties.

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

@@ -9,13 +9,19 @@
 import javascript
 private import internal.FlowSteps
 
+private class PropertyName extends string {
+  PropertyName() { this = any(DataFlow::PropRef pr).getPropertyName() }
+}
+
 /**
  * A description of a step on an inter-procedural data flow path.
  */
 private newtype TStepSummary =
   LevelStep() or
   CallStep() or
-  ReturnStep()
+  ReturnStep() or
+  StoreStep(PropertyName prop) or
+  LoadStep(PropertyName prop)
 
 /**
  * INTERNAL: Use `TypeTracker` or `TypeBackTracker` instead.
@@ -64,6 +70,14 @@ module StepSummary {
       // Flow through an instance field between members of the same class
       DataFlow::localFieldStep(predNode, succ) and
       summary = LevelStep()
+      or
+      exists(string prop |
+        basicStoreStep(predNode, succ, prop) and
+        summary = StoreStep(prop)
+        or
+        loadStep(predNode, succ, prop) and
+        summary = LoadStep(prop)
+      )
     )
   }
 
@@ -73,8 +87,22 @@ module StepSummary {
    * Appends a step summary onto a type-tracking summary.
    */
   TypeTracker append(TypeTracker type, StepSummary summary) {
-    not (type.hasCall() = true and summary.hasReturn() = true) and
-    result.hasCall() = type.hasCall().booleanOr(summary.hasCall())
+    exists(boolean hadCall, boolean hasCall, string oldProp, string newProp |
+      hadCall = type.hasCall() and
+      oldProp = type.getProp()
+    |
+      not (hadCall = true and summary.hasReturn() = true) and
+      hasCall = hadCall.booleanOr(summary.hasCall()) and
+      (
+        if summary instanceof StoreStep
+        then oldProp = "" and summary = StoreStep(newProp)
+        else
+          if summary instanceof LoadStep
+          then summary = LoadStep(oldProp) and newProp = ""
+          else newProp = oldProp
+      ) and
+      result = MkTypeTracker(hasCall, newProp)
+    )
   }
 
   /**
@@ -83,12 +111,27 @@ module StepSummary {
    * Prepends a step summary before a backwards type-tracking summary.
    */
   TypeBackTracker prepend(StepSummary summary, TypeBackTracker type) {
-    not (type.hasReturn() = true and summary.hasCall() = true) and
-    result.hasReturn() = type.hasReturn().booleanOr(summary.hasReturn())
+    exists(boolean hadReturn, boolean hasReturn, string oldProp, string newProp |
+      hadReturn = type.hasReturn() and
+      oldProp = type.getProp()
+    |
+      not (hadReturn = true and summary.hasCall() = true) and
+      hasReturn = hadReturn.booleanOr(summary.hasReturn()) and
+      (
+        if summary instanceof StoreStep
+        then summary = StoreStep(oldProp) and newProp = ""
+        else
+          if summary instanceof LoadStep
+          then oldProp = "" and summary = LoadStep(newProp)
+          else newProp = oldProp
+      ) and
+      result = MkTypeBackTracker(hasReturn, newProp)
+    )
   }
 }
 
-private newtype TTypeTracker = MkTypeTracker(Boolean hasCall)
+private newtype TTypeTracker =
+  MkTypeTracker(Boolean hasCall, string prop) { prop = "" or prop instanceof PropertyName }
 
 /**
  * EXPERIMENTAL.
@@ -112,7 +155,7 @@ private newtype TTypeTracker = MkTypeTracker(Boolean hasCall)
  *   )
  * }
  *
- * DataFlow::SourceNode myType() { result = myType(_) }
+ * DataFlow::SourceNode myType() { result = myType(DataFlow::TypeTracker::end()) }
  * ```
  *
  * To track values backwards, which can be useful for tracking
@@ -121,18 +164,24 @@ private newtype TTypeTracker = MkTypeTracker(Boolean hasCall)
 class TypeTracker extends TTypeTracker {
   Boolean hasCall;
 
-  TypeTracker() { this = MkTypeTracker(hasCall) }
+  string prop;
+
+  TypeTracker() { this = MkTypeTracker(hasCall, prop) }
 
   string toString() {
-    hasCall = true and result = "type tracker with call steps"
-    or
-    hasCall = false and result = "type tracker without call steps"
+    exists(string withCall, string withProp |
+      (if hasCall = true then withCall = "with" else withCall = "without") and
+      (if prop != "" then withProp = " with property " + prop else withProp = "") and
+      result = "type tracker " + withCall + " call steps" + withProp
+    )
   }
 
   /**
    * Holds if this is the starting point of type tracking.
    */
-  predicate start() { hasCall = false }
+  predicate start() { hasCall = false and prop = "" }
+
+  predicate end() { prop = "" }
 
   /**
    * INTERNAL. DO NOT USE.
@@ -140,9 +189,16 @@ class TypeTracker extends TTypeTracker {
    * Holds if this type has been tracked into a call.
    */
   boolean hasCall() { result = hasCall }
+
+  string getProp() { result = prop }
 }
 
-private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn)
+module TypeTracker {
+  TypeTracker end() { result.end() }
+}
+
+private newtype TTypeBackTracker =
+  MkTypeBackTracker(Boolean hasReturn, string prop) { prop = "" or prop instanceof PropertyName }
 
 /**
  * EXPERIMENTAL.
@@ -168,24 +224,30 @@ private newtype TTypeBackTracker = MkTypeBackTracker(Boolean hasReturn)
  *   )
  * }
  *
- * DataFlow::SourceNode myCallback() { result = myCallback(_) }
+ * DataFlow::SourceNode myCallback() { result = myCallback(DataFlow::TypeBackTracker::end()) }
  * ```
  */
 class TypeBackTracker extends TTypeBackTracker {
   Boolean hasReturn;
 
-  TypeBackTracker() { this = MkTypeBackTracker(hasReturn) }
+  string prop;
+
+  TypeBackTracker() { this = MkTypeBackTracker(hasReturn, prop) }
 
   string toString() {
-    hasReturn = true and result = "type back-tracker with return steps"
-    or
-    hasReturn = false and result = "type back-tracker without return steps"
+    exists(string withReturn, string withProp |
+      (if hasReturn = true then withReturn = "with" else withReturn = "without") and
+      (if prop != "" then withProp = " with property " + prop else withProp = "") and
+      result = "type back-tracker " + withReturn + " return steps" + withProp
+    )
   }
 
   /**
    * Holds if this is the starting point of type tracking.
    */
-  predicate start() { hasReturn = false }
+  predicate start() { hasReturn = false and prop = "" }
+
+  predicate end() { prop = "" }
 
   /**
    * INTERNAL. DO NOT USE.
@@ -193,4 +255,10 @@ class TypeBackTracker extends TTypeBackTracker {
    * Holds if this type has been back-tracked into a call through return edge.
    */
   boolean hasReturn() { result = hasReturn }
+
+  string getProp() { result = prop }
+}
+
+module TypeBackTracker {
+  TypeBackTracker end() { result.end() }
 }

javascript/ql/src/semmle/javascript/frameworks/Connect.qll

@@ -118,7 +118,7 @@ module Connect {
     }
 
     override DataFlow::SourceNode getARouteHandler() {
-      result = getARouteHandler(_)
+      result = getARouteHandler(DataFlow::TypeBackTracker::end())
     }
 
     private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {

javascript/ql/src/semmle/javascript/frameworks/Electron.qll

@@ -69,7 +69,7 @@ module Electron {
    * A data flow node whose value may originate from a browser object instantiation.
    */
   private class BrowserObjectByFlow extends BrowserObject {
-    BrowserObjectByFlow() { browserObject(_).flowsTo(this) }
+    BrowserObjectByFlow() { browserObject(DataFlow::TypeTracker::end()).flowsTo(this) }
   }
 
   /**

javascript/ql/src/semmle/javascript/frameworks/Express.qll

@@ -111,7 +111,7 @@ module Express {
     Expr getLastRouteHandlerExpr() { result = max(int i | | getRouteHandlerExpr(i) order by i) }
 
     override DataFlow::SourceNode getARouteHandler() {
-      result = getARouteHandler(_)
+      result = getARouteHandler(DataFlow::TypeBackTracker::end())
     }
 
     private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {

javascript/ql/src/semmle/javascript/frameworks/HTTP.qll

@@ -282,7 +282,7 @@ module HTTP {
        */
       abstract RouteHandler getRouteHandler();
 
-      predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(_).flowsTo(nd) }
+      predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(DataFlow::TypeTracker::end()).flowsTo(nd) }
 
       private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) {
         t.start() and
@@ -303,7 +303,7 @@ module HTTP {
        */
       abstract RouteHandler getRouteHandler();
 
-      predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(_).flowsTo(nd) }
+      predicate flowsTo(DataFlow::Node nd) { flowsToSourceNode(DataFlow::TypeTracker::end()).flowsTo(nd) }
 
       private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) {
         t.start() and

javascript/ql/src/semmle/javascript/frameworks/Hapi.qll

@@ -193,7 +193,7 @@ module Hapi {
     }
 
     override DataFlow::SourceNode getARouteHandler() {
-      result = getARouteHandler(_)
+      result = getARouteHandler(DataFlow::TypeBackTracker::end())
     }
 
     private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {

javascript/ql/src/semmle/javascript/frameworks/Koa.qll

@@ -79,7 +79,7 @@ module Koa {
     RouteHandler getRouteHandler() { result = rh }
 
     predicate flowsTo(DataFlow::Node nd) {
-      flowsToSourceNode(_).flowsTo(nd)
+      flowsToSourceNode(DataFlow::TypeTracker::end()).flowsTo(nd)
     }
 
     private DataFlow::SourceNode flowsToSourceNode(DataFlow::TypeTracker t) {

javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll

@@ -189,7 +189,7 @@ module NodeJSLib {
     }
 
     override DataFlow::SourceNode getARouteHandler() {
-      result = getARouteHandler(_)
+      result = getARouteHandler(DataFlow::TypeBackTracker::end())
     }
 
     private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {

javascript/ql/src/semmle/javascript/frameworks/SocketIO.qll

@@ -61,7 +61,7 @@ module SocketIO {
   class ServerNode extends DataFlow::SourceNode {
     ServerObject srv;
 
-    ServerNode() { this = server(srv, _) }
+    ServerNode() { this = server(srv, DataFlow::TypeTracker::end()) }
 
     /** Gets the server to which this node refers. */
     ServerObject getServer() { result = srv }
@@ -130,7 +130,7 @@ module SocketIO {
   class NamespaceNode extends DataFlow::SourceNode {
     NamespaceObject ns;
 
-    NamespaceNode() { this = namespace(ns, _) }
+    NamespaceNode() { this = namespace(ns, DataFlow::TypeTracker::end()) }
 
     /** Gets the namespace to which this node refers. */
     NamespaceObject getNamespace() { result = ns }
@@ -194,7 +194,7 @@ module SocketIO {
   class SocketNode extends DataFlow::SourceNode {
     NamespaceObject ns;
 
-    SocketNode() { this = socket(ns, _) }
+    SocketNode() { this = socket(ns, DataFlow::TypeTracker::end()) }
 
     /** Gets the namespace to which this socket belongs. */
     NamespaceObject getNamespace() { result = ns }
@@ -417,7 +417,7 @@ module SocketIOClient {
   class SocketNode extends DataFlow::SourceNode {
     DataFlow::InvokeNode invk;
 
-    SocketNode() { this = socket(invk, _) }
+    SocketNode() { this = socket(invk, DataFlow::TypeTracker::end()) }
 
     /** Gets the path of the namespace this socket belongs to, if it can be determined. */
     string getNamespacePath() {

javascript/ql/test/library-tests/TypeTracking/ClassStyle.ql

@@ -6,86 +6,68 @@ string chainableMethod() {
 }
 
 class ApiObject extends DataFlow::NewNode {
-  ApiObject() {
-    this = DataFlow::moduleImport("@test/myapi").getAnInstantiation()
-  }
+  ApiObject() { this = DataFlow::moduleImport("@test/myapi").getAnInstantiation() }
 
   DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
     t.start() and
     result = this
     or
     t.start() and
-    result = ref(_).getAMethodCall(chainableMethod())
+    result = ref(DataFlow::TypeTracker::end()).getAMethodCall(chainableMethod())
     or
-    exists(DataFlow::TypeTracker t2 |
-      result = ref(t2).track(t2, t)
-    )
+    exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
   }
 
-  DataFlow::SourceNode ref() {
-    result = ref(_)
-  }
+  DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
 }
 
 class Connection extends DataFlow::SourceNode {
   ApiObject api;
 
-  Connection() {
-    this = api.ref().getAMethodCall("createConnection")
-  }
+  Connection() { this = api.ref().getAMethodCall("createConnection") }
 
   DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
     t.start() and
     result = this
     or
-    exists(DataFlow::TypeTracker t2 |
-      result = ref(t2).track(t2, t)
-    )
+    exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
   }
 
-  DataFlow::SourceNode ref() {
-    result = ref(_)
-  }
+  DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
 
   DataFlow::SourceNode getACallbackNode(DataFlow::TypeBackTracker t) {
     t.start() and
     result = ref().getAMethodCall("getData").getArgument(0).getALocalSource()
     or
-    exists(DataFlow::TypeBackTracker t2 |
-      result = getACallbackNode(t2).backtrack(t2, t)
-    )
+    exists(DataFlow::TypeBackTracker t2 | result = getACallbackNode(t2).backtrack(t2, t))
   }
 
   DataFlow::FunctionNode getACallback() {
-    result = getACallbackNode(_).getAFunctionValue()
+    result = getACallbackNode(DataFlow::TypeBackTracker::end()).getAFunctionValue()
   }
 }
 
 class DataValue extends DataFlow::SourceNode {
   Connection connection;
 
-  DataValue() {
-    this = connection.getACallback().getParameter(0)
-  }
+  DataValue() { this = connection.getACallback().getParameter(0) }
 
   DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
     t.start() and
     result = this
     or
-    exists(DataFlow::TypeTracker t2 |
-      result = ref(t2).track(t2, t)
-    )
+    exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
   }
 
-  DataFlow::SourceNode ref() {
-    result = ref(_)
-  }
+  DataFlow::SourceNode ref() { result = ref(DataFlow::TypeTracker::end()) }
 }
 
 query DataFlow::SourceNode test_ApiObject() { result = any(ApiObject obj).ref() }
 
 query DataFlow::SourceNode test_Connection() { result = any(Connection c).ref() }
 
-query DataFlow::SourceNode test_DataCallback() { result = any(Connection c).getACallbackNode(_) }
+query DataFlow::SourceNode test_DataCallback() {
+  result = any(Connection c).getACallbackNode(DataFlow::TypeBackTracker::end())
+}
 
 query DataFlow::SourceNode test_DataValue() { result = any(DataValue v).ref() }

javascript/ql/test/library-tests/TypeTracking/PredicateStyle.ql

@@ -10,52 +10,38 @@ DataFlow::SourceNode apiObject(DataFlow::TypeTracker t) {
   result = DataFlow::moduleImport("@test/myapi").getAnInstantiation()
   or
   t.start() and
-  result = apiObject(_).getAMethodCall(chainableMethod())
+  result = apiObject(DataFlow::TypeTracker::end()).getAMethodCall(chainableMethod())
   or
-  exists(DataFlow::TypeTracker t2 |
-    result = apiObject(t2).track(t2, t)
-  )
+  exists(DataFlow::TypeTracker t2 | result = apiObject(t2).track(t2, t))
 }
 
-query DataFlow::SourceNode apiObject() {
-  result = apiObject(_)
-}
+query DataFlow::SourceNode apiObject() { result = apiObject(DataFlow::TypeTracker::end()) }
 
 query DataFlow::SourceNode connection(DataFlow::TypeTracker t) {
   t.start() and
   result = apiObject().getAMethodCall("createConnection")
   or
-  exists(DataFlow::TypeTracker t2 |
-    result = connection(t2).track(t2, t)
-  )
+  exists(DataFlow::TypeTracker t2 | result = connection(t2).track(t2, t))
 }
 
-DataFlow::SourceNode connection() {
-  result = connection(_)
-}
+DataFlow::SourceNode connection() { result = connection(DataFlow::TypeTracker::end()) }
 
 DataFlow::SourceNode dataCallback(DataFlow::TypeBackTracker t) {
   t.start() and
   result = connection().getAMethodCall("getData").getArgument(0).getALocalSource()
   or
-  exists(DataFlow::TypeBackTracker t2 |
-    result = dataCallback(t2).backtrack(t2, t)
-  )
+  exists(DataFlow::TypeBackTracker t2 | result = dataCallback(t2).backtrack(t2, t))
 }
 
 query DataFlow::SourceNode dataCallback() {
-  result = dataCallback(_)
+  result = dataCallback(DataFlow::TypeBackTracker::end())
 }
 
 DataFlow::SourceNode dataValue(DataFlow::TypeTracker t) {
   t.start() and
   result = dataCallback().getAFunctionValue().getParameter(0)
   or
-  exists(DataFlow::TypeTracker t2 |
-    result = dataValue(t2).track(t2, t)
-  )
+  exists(DataFlow::TypeTracker t2 | result = dataValue(t2).track(t2, t))
 }
 
-query DataFlow::SourceNode dataValue() {
-  result = dataValue(_)
-}
+query DataFlow::SourceNode dataValue() { result = dataValue(DataFlow::TypeTracker::end()) }

javascript/ql/test/library-tests/frameworks/HTTP-heuristics/RouteHandler.expected

@@ -11,6 +11,8 @@
 | src/exported-handler.js:1:19:1:55 | functio ... res) {} |
 | src/exported-middleware-attacher-2.js:2:13:2:32 | function(req, res){} |
 | src/exported-middleware-attacher.js:2:13:2:32 | function(req, res){} |
+| src/handler-in-property.js:5:14:5:33 | function(req, res){} |
+| src/handler-in-property.js:12:18:12:37 | function(req, res){} |
 | src/middleware-attacher-getter.js:4:17:4:36 | function(req, res){} |
 | src/middleware-attacher-getter.js:19:19:19:38 | function(req, res){} |
 | src/middleware-attacher.js:3:13:3:32 | function(req, res){} |

javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected

@@ -1,7 +1,5 @@
 | src/bound-handler.js:4:1:4:28 | functio ...  res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/bound-handler.js:9:12:9:31 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/handler-in-property.js:5:14:5:33 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/handler-in-property.js:12:18:12:37 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/hapi.js:1:1:1:30 | functio ... t, h){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/iterated-handlers.js:4:2:4:22 | functio ...  res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/middleware-attacher-getter.js:29:32:29:51 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |