JS: Refactor RequestInputAccess to use source nodes

Author: asgerf

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

@@ -461,28 +461,21 @@ module Express {
     string kind;
 
     RequestInputAccess() {
-      exists(DataFlow::Node request | request = DataFlow::valueNode(rh.getARequestExpr()) |
+      exists(DataFlow::SourceNode request | request = rh.getARequestSource().ref() |
         kind = "parameter" and
         (
-          this.(DataFlow::MethodCallNode).calls(request, "param")
+          this = request.getAMethodCall("param")
           or
-          exists(DataFlow::PropRead base, string propName |
-            // `req.params.name` or `req.query.name`
-            base.accesses(request, propName) and
-            this = base.getAPropertyReference(_)
-          |
-            propName = "params" or
-            propName = "query"
-          )
+          this = request.getAPropertyRead(["params", "query"]).getAPropertyRead()
         )
         or
         // `req.originalUrl`
         kind = "url" and
-        this.(DataFlow::PropRef).accesses(request, "originalUrl")
+        this = request.getAPropertyRead("originalUrl")
         or
         // `req.cookies`
         kind = "cookie" and
-        this.(DataFlow::PropRef).accesses(request, "cookies")
+        this = request.getAPropertyRead("cookies")
       )
       or
       kind = "body" and

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

@@ -204,6 +204,24 @@ module HTTP {
      */
     abstract HeaderDefinition getAResponseHeader(string name);
 
+    /**
+     * Gets a request object originating from this route handler.
+     *
+     * Use `RequestSource.ref()` to get reference to this request object.
+     */
+    final Servers::RequestSource getARequestSource() {
+      result.getRouteHandler() = this
+    }
+
+    /**
+     * Gets a request object originating from this route handler.
+     *
+     * Use `RequestSource.ref()` to get reference to this request object.
+     */
+    final Servers::ResponseSource getAResponseSource() {
+      result.getRouteHandler() = this
+    }
+
     /**
      * Gets an expression that contains a request object handled
      * by this handler.
@@ -296,14 +314,21 @@ module HTTP {
        */
       abstract RouteHandler getRouteHandler();
 
-      predicate flowsTo(DataFlow::Node nd) { ref(DataFlow::TypeTracker::end()).flowsTo(nd) }
+      /** DEPRECATED. Use `ref().flowsTo()` instead. */
+      deprecated
+      predicate flowsTo(DataFlow::Node nd) { ref().flowsTo(nd) }
 
       private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
         t.start() and
         result = this
         or
         exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
       }
+
+      /** Gets a `SourceNode` that refers to this request object. */
+      DataFlow::SourceNode ref() {
+        result = ref(DataFlow::TypeTracker::end())
+      }
     }
 
     /**
@@ -317,14 +342,20 @@ module HTTP {
        */
       abstract RouteHandler getRouteHandler();
 
-      predicate flowsTo(DataFlow::Node nd) { ref(DataFlow::TypeTracker::end()).flowsTo(nd) }
+      /** DEPRECATED. Use `ref().flowsTo()` instead. */
+      predicate flowsTo(DataFlow::Node nd) { ref().flowsTo(nd) }
 
       private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
         t.start() and
         result = this
         or
         exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
       }
+
+      /** Gets a `SourceNode` that refers to this response object. */
+      DataFlow::SourceNode ref() {
+        result = ref(DataFlow::TypeTracker::end())
+      }
     }
 
     /**
@@ -333,7 +364,7 @@ module HTTP {
     class StandardRequestExpr extends RequestExpr {
       RequestSource src;
 
-      StandardRequestExpr() { src.flowsTo(DataFlow::valueNode(this)) }
+      StandardRequestExpr() { src.ref().flowsTo(DataFlow::valueNode(this)) }
 
       override RouteHandler getRouteHandler() { result = src.getRouteHandler() }
     }
@@ -344,7 +375,7 @@ module HTTP {
     class StandardResponseExpr extends ResponseExpr {
       ResponseSource src;
 
-      StandardResponseExpr() { src.flowsTo(DataFlow::valueNode(this)) }
+      StandardResponseExpr() { src.ref().flowsTo(DataFlow::valueNode(this)) }
 
       override RouteHandler getRouteHandler() { result = src.getRouteHandler() }
     }

javascript/ql/test/library-tests/frameworks/Express/tests.expected

@@ -279,6 +279,8 @@ test_RequestInputAccess
 | src/express3.js:5:35:5:50 | req.param("val") | parameter | src/express3.js:4:23:7:1 | functio ... al");\\n} |
 | src/express4.js:5:9:5:11 | foo | parameter | src/express4.js:4:23:9:1 | functio ... ic1);\\n} |
 | src/express4.js:5:14:5:21 | bar: baz | parameter | src/express4.js:4:23:9:1 | functio ... ic1);\\n} |
+| src/express4.js:6:18:6:31 | req.query[foo] | parameter | src/express4.js:4:23:9:1 | functio ... ic1);\\n} |
+| src/express4.js:7:18:7:39 | req.que ... hing()] | parameter | src/express4.js:4:23:9:1 | functio ... ic1);\\n} |
 | src/express.js:5:16:5:34 | req.param("target") | parameter | src/express.js:4:23:9:1 | functio ... res);\\n} |
 | src/express.js:6:26:6:44 | req.param("target") | parameter | src/express.js:4:23:9:1 | functio ... res);\\n} |
 | src/express.js:23:3:23:10 | req.body | body | src/express.js:22:30:32:1 | functio ... ar');\\n} |