Merge pull request #1526 from xiemaisi/js/remove-TrackedExpr

Approved by esben-semmle

javascript/ql/src/meta/analysis-quality/CandidateTracking.qll

@@ -0,0 +1,16 @@
+/**
+ * Provides an auxiliary predicate shared among the unpromoted-candidate queries.
+ */
+
+import javascript
+
+/**
+ * Gets a source node to which `cand` may flow inter-procedurally, with `t` tracking
+ * the state of flow.
+ */
+DataFlow::SourceNode track(HTTP::RouteHandlerCandidate cand, DataFlow::TypeTracker t) {
+  t.start() and
+  result = cand
+  or
+  exists(DataFlow::TypeTracker t2 | result = track(cand, t2).track(t2, t))
+}

javascript/ql/src/meta/analysis-quality/UnpromotedRouteHandlerCandidate.ql

@@ -9,12 +9,13 @@
  */
 
 import javascript
+import CandidateTracking
 
 from HTTP::RouteHandlerCandidate rh
 where
   not rh instanceof HTTP::RouteHandler and
   not exists(HTTP::RouteSetupCandidate setup |
-    rh.(DataFlow::TrackedNode).flowsTo(setup.getARouteHandlerArg())
+    track(rh, DataFlow::TypeTracker::end()).flowsTo(setup.getARouteHandlerArg())
   )
 select rh,
   "A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`."

javascript/ql/src/meta/analysis-quality/UnpromotedRouteSetupCandidate.ql

@@ -9,12 +9,13 @@
  */
 
 import javascript
+import CandidateTracking
 
 from HTTP::RouteSetupCandidate setup
 where
   not setup.asExpr() instanceof HTTP::RouteSetup and
   exists(HTTP::RouteHandlerCandidate rh |
-    rh.(DataFlow::TrackedNode).flowsTo(setup.getARouteHandlerArg())
+    track(rh, DataFlow::TypeTracker::end()).flowsTo(setup.getARouteHandlerArg())
   )
 select setup,
   "A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`."

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

@@ -657,7 +657,7 @@ module Express {
   /**
    * An Express server application.
    */
-  private class Application extends HTTP::ServerDefinition, DataFlow::TrackedExpr {
+  private class Application extends HTTP::ServerDefinition {
     Application() { this = appCreation().asExpr() }
 
     /**
@@ -671,9 +671,23 @@ module Express {
   /**
    * An Express router.
    */
-  class RouterDefinition extends InvokeExpr, DataFlow::TrackedExpr {
+  class RouterDefinition extends InvokeExpr {
     RouterDefinition() { this = routerCreation().asExpr() }
 
+    private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
+      t.start() and
+      result = DataFlow::exprNode(this)
+      or
+      exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
+    }
+
+    /**
+     * Holds if `sink` may refer to this router.
+     */
+    predicate flowsTo(Expr sink) {
+      ref(DataFlow::TypeTracker::end()).flowsToExpr(sink)
+    }
+
     /**
      * Gets a `RouteSetup` that was used for setting up a route on this router.
      */

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

@@ -248,8 +248,20 @@ module HTTP {
     /**
      * A standard server definition.
      */
-    abstract class StandardServerDefinition extends ServerDefinition, DataFlow::TrackedExpr {
+    abstract class StandardServerDefinition extends ServerDefinition {
       override RouteHandler getARouteHandler() { result.(StandardRouteHandler).getServer() = this }
+
+      private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
+        t.start() and
+        result = DataFlow::exprNode(this)
+        or
+        exists(DataFlow::TypeTracker t2 | result = ref(t2).track(t2, t))
+      }
+
+      /**
+       * Holds if `sink` may refer to this server definition.
+       */
+      predicate flowsTo(Expr sink) { ref(DataFlow::TypeTracker::end()).flowsToExpr(sink) }
     }
 
     /**

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

@@ -9,8 +9,7 @@ module Restify {
   /**
    * An expression that creates a new Restify server.
    */
-  class ServerDefinition extends HTTP::Servers::StandardServerDefinition, CallExpr,
-    DataFlow::TrackedExpr {
+  class ServerDefinition extends HTTP::Servers::StandardServerDefinition, CallExpr {
     ServerDefinition() {
       // `server = restify.createServer()`
       this = DataFlow::moduleMember("restify", "createServer").getACall().asExpr()

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

@@ -3,9 +3,6 @@
 | 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`. |
-| src/nodejs.js:5:22:5:41 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/nodejs.js:11:23:11:42 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/nodejs.js:12:25:12:44 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/route-objects.js:7:19:7:38 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/route-objects.js:8:12:10:5 | (req, res) {\\n\\n    } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/route-objects.js:20:16:22:9 | (req, r ...       } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
@@ -13,20 +10,8 @@
 | src/route-objects.js:40:12:42:5 | (req, res) {\\n\\n    } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/route-objects.js:50:12:52:5 | (req, res) {\\n\\n    } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/route-objects.js:56:12:58:5 | functio ... ;\\n    } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:6:32:6:52 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:8:32:8:61 | functio ... nse) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:30:36:30:56 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:33:18:33:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:34:18:34:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:37:5:37:25 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:38:5:38:25 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:43:18:43:38 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/tst.js:46:1:46:23 | functio ... res) {} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/tst.js:52:1:54:1 | functio ... req()\\n} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/tst.js:61:1:63:1 | functio ... turn;\\n} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/tst.js:70:5:72:5 | functio ... \\n\\n    } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:79:5:81:5 | functio ... \\n\\n    } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:84:5:86:5 | functio ... \\n\\n    } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/tst.js:109:16:111:9 | functio ...       } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:124:16:126:9 | functio ...       } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
-| src/tst.js:132:16:134:9 | functio ...       } | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |

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

@@ -0,0 +1,17 @@
+| src/nodejs.js:5:1:5:42 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/nodejs.js:11:1:11:43 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/nodejs.js:12:1:12:45 | unknown ... res){}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:6:1:6:53 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:8:1:8:62 | someOth ... se) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:30:1:30:57 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:32:1:34:39 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:36:1:39:2 | someOth ... ) {}\\n]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:41:1:43:39 | someOth ... es) {}) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:87:5:87:57 | unknown ... cSetup) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:96:5:96:36 | unknown ... h', rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:98:5:98:38 | unknown ... , [rh]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:104:5:104:45 | unknown ... wn, rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:137:5:137:57 | unknown ... cSetup) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:149:5:149:36 | unknown ... h', rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:151:5:151:38 | unknown ... , [rh]) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |
+| src/tst.js:157:5:157:45 | unknown ... wn, rh) | A `RouteSetupCandidate` that did not get promoted to `RouteSetup`, and it is using at least one `RouteHandlerCandidate`. |