don't use astNode in StandardHeaderDefinition

Author: erik-krogh

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

@@ -674,12 +674,12 @@ module Express {
   /**
    * Holds if `e` is an HTTP request object.
    */
-  predicate isRequest(Expr e) { any(RouteHandler rh).getARequestExpr() = e }
+  predicate isRequest(Expr e) { any(RouteHandler rh).getARequestExpr() = e } // TODO: DataFlow::Node
 
   /**
    * Holds if `e` is an HTTP response object.
    */
-  predicate isResponse(Expr e) { any(RouteHandler rh).getAResponseExpr() = e }
+  predicate isResponse(Expr e) { any(RouteHandler rh).getAResponseExpr() = e } // TODO: DataFlow::Node
 
   /**
    * An access to the HTTP request body.
@@ -689,9 +689,11 @@ module Express {
   }
 
   abstract private class HeaderDefinition extends HTTP::Servers::StandardHeaderDefinition {
-    HeaderDefinition() { isResponse(astNode.getReceiver()) }
+    HeaderDefinition() { isResponse(this.getReceiver().asExpr()) }
 
-    override RouteHandler getRouteHandler() { astNode.getReceiver() = result.getAResponseExpr() }
+    override RouteHandler getRouteHandler() {
+      this.getReceiver().asExpr() = result.getAResponseExpr()
+    }
   }
 
   /**
@@ -713,8 +715,8 @@ module Express {
    */
   private class SetOneHeader extends HeaderDefinition {
     SetOneHeader() {
-      astNode.getMethodName() = any(string n | n = "set" or n = "header") and
-      astNode.getNumArgument() = 2
+      this.getMethodName() = any(string n | n = "set" or n = "header") and
+      this.getNumArgument() = 2
     }
   }
 
@@ -744,9 +746,9 @@ module Express {
 
     override RouteHandler getRouteHandler() { result = response.getRouteHandler() }
 
-    override Expr getNameExpr() {
+    override DataFlow::Node getNameNode() {
       exists(DataFlow::PropWrite write | this.getAHeaderSource().getAPropertyWrite() = write |
-        result = write.getPropertyNameExpr()
+        result = write.getPropertyNameExpr().flow()
       )
     }
   }
@@ -755,7 +757,7 @@ module Express {
    * An invocation of the `append` method on an HTTP response object.
    */
   private class AppendHeader extends HeaderDefinition {
-    AppendHeader() { astNode.getMethodName() = "append" }
+    AppendHeader() { this.getMethodName() = "append" }
   }
 
   /**

javascript/ql/lib/semmle/javascript/frameworks/Fastify.qll

@@ -401,9 +401,9 @@ module Fastify {
 
     override RouteHandler getRouteHandler() { result = rh }
 
-    override Expr getNameExpr() {
+    override DataFlow::Node getNameNode() {
       exists(DataFlow::PropWrite write | this.getAHeaderSource().getAPropertyWrite() = write |
-        result = write.getPropertyNameExpr()
+        result = write.getPropertyNameExpr().flow()
       )
     }
   }

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

@@ -68,12 +68,16 @@ module HTTP {
     /**
      * Holds if the header with (lower-case) name `headerName` is set to the value of `headerValue`.
      */
-    abstract predicate definesExplicitly(string headerName, Expr headerValue);
+    abstract predicate definesExplicitly(string headerName, Expr headerValue); // TODO: DataFlow::Node.
 
     /**
+     * DEPRECATED: Use `getNameNode()` instead.
      * Returns the expression used to compute the header name.
      */
-    abstract Expr getNameExpr();
+    deprecated Expr getNameExpr() { result = this.getNameNode().asExpr() }
+
+    /** Returns the expression used to compute the header name. */
+    abstract DataFlow::Node getNameNode();
   }
 
   /**
@@ -201,13 +205,13 @@ module HTTP {
      * Gets an expression that contains a request object handled
      * by this handler.
      */
-    RequestExpr getARequestExpr() { result.getRouteHandler() = this }
+    RequestExpr getARequestExpr() { result.getRouteHandler() = this } // TODO: DataFlow::Node
 
     /**
      * Gets an expression that contains a response object provided
      * by this handler.
      */
-    ResponseExpr getAResponseExpr() { result.getRouteHandler() = this }
+    ResponseExpr getAResponseExpr() { result.getRouteHandler() = this } // TODO: DataFlow::Node
   }
 
   /**
@@ -238,6 +242,7 @@ module HTTP {
    * An expression that may contain a request object.
    */
   abstract class RequestExpr extends Expr {
+    // TODO: DataFlow::Node
     /**
      * Gets the route handler that handles this request.
      */
@@ -248,6 +253,7 @@ module HTTP {
    * An expression that may contain a response object.
    */
   abstract class ResponseExpr extends Expr {
+    // TODO: DataFlow::Node
     /**
      * Gets the route handler that handles this request.
      */
@@ -376,15 +382,14 @@ module HTTP {
     /**
      * A standard header definition.
      */
-    abstract class StandardHeaderDefinition extends ExplicitHeaderDefinition, DataFlow::ValueNode {
-      override MethodCallExpr astNode;
-
+    abstract class StandardHeaderDefinition extends ExplicitHeaderDefinition,
+      DataFlow::MethodCallNode {
       override predicate definesExplicitly(string headerName, Expr headerValue) {
-        headerName = this.getNameExpr().getStringValue().toLowerCase() and
-        headerValue = astNode.getArgument(1)
+        headerName = this.getNameNode().getStringValue().toLowerCase() and
+        headerValue = this.getArgument(1).asExpr()
       }
 
-      override Expr getNameExpr() { result = astNode.getArgument(0) }
+      override DataFlow::Node getNameNode() { result = this.getArgument(0) }
     }
 
     /**

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

@@ -74,13 +74,15 @@ module Hapi {
     override RouteHandler getRouteHandler() { result = rh }
   }
 
+  // TODO: DataFlow::Node
   /**
    * A Hapi response expression.
    */
   class ResponseExpr extends HTTP::Servers::StandardResponseExpr {
     override ResponseSource src;
   }
 
+  // TODO: DataFlow::Node
   /**
    * An Hapi request expression.
    */
@@ -175,7 +177,7 @@ module Hapi {
 
     HeaderDefinition() {
       // request.response.header('Cache-Control', 'no-cache')
-      astNode.calls(res, "header")
+      this.calls(res.flow(), "header")
     }
 
     override RouteHandler getRouteHandler() { result = res.getRouteHandler() }

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

@@ -24,10 +24,10 @@ module Koa {
 
     HeaderDefinition() {
       // ctx.set('Cache-Control', 'no-cache');
-      astNode.calls(rh.getAResponseOrContextExpr(), "set")
+      this.calls(rh.getAResponseOrContextExpr().flow(), "set")
       or
       // ctx.response.header('Cache-Control', 'no-cache')
-      astNode.calls(rh.getAResponseExpr(), "header")
+      this.calls(rh.getAResponseExpr().flow(), "header")
     }
 
     override RouteHandler getRouteHandler() { result = rh }
@@ -59,6 +59,7 @@ module Koa {
      * object of a route handler invocation.
      */
     Expr getAResponseOrContextExpr() {
+      // TODO: DataFlow::Node
       result = this.getAResponseExpr() or result = this.getAContextExpr()
     }
 

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

@@ -259,7 +259,7 @@ module NodeJSLib {
   abstract private class HeaderDefinition extends HTTP::Servers::StandardHeaderDefinition {
     ResponseExpr r;
 
-    HeaderDefinition() { astNode.getReceiver() = r }
+    HeaderDefinition() { this.getReceiver().asExpr() = r }
 
     override HTTP::RouteHandler getRouteHandler() { result = r.getRouteHandler() }
   }
@@ -268,22 +268,22 @@ module NodeJSLib {
    * A call to the `setHeader` method of an HTTP response.
    */
   private class SetHeader extends HeaderDefinition {
-    SetHeader() { astNode.getMethodName() = "setHeader" }
+    SetHeader() { this.getMethodName() = "setHeader" }
   }
 
   /**
    * A call to the `writeHead` method of an HTTP response.
    */
   private class WriteHead extends HeaderDefinition {
     WriteHead() {
-      astNode.getMethodName() = "writeHead" and
-      astNode.getNumArgument() >= 1
+      this.getMethodName() = "writeHead" and
+      this.getNumArgument() >= 1
     }
 
     override predicate definesExplicitly(string headerName, Expr headerValue) {
-      astNode.getNumArgument() > 1 and
+      this.getNumArgument() > 1 and
       exists(DataFlow::SourceNode headers, string header |
-        headers.flowsToExpr(astNode.getLastArgument()) and
+        headers.flowsTo(this.getLastArgument()) and
         headers.hasPropertyWrite(header, DataFlow::valueNode(headerValue)) and
         headerName = header.toLowerCase()
       )

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

@@ -68,13 +68,15 @@ module Restify {
     override RouteHandler getRouteHandler() { result = rh }
   }
 
+  // TODO: DataFlow::Node
   /**
    * A Node.js HTTP response provided by Restify.
    */
   class ResponseExpr extends NodeJSLib::ResponseExpr {
     ResponseExpr() { src instanceof ResponseSource }
   }
 
+  // TODO: DataFlow::Node
   /**
    * A Node.js HTTP request provided by Restify.
    */
@@ -128,11 +130,13 @@ module Restify {
   private class HeaderDefinition extends HTTP::Servers::StandardHeaderDefinition {
     HeaderDefinition() {
       // response.header('Cache-Control', 'no-cache')
-      astNode.getReceiver() instanceof ResponseExpr and
-      astNode.getMethodName() = "header"
+      this.getReceiver().asExpr() instanceof ResponseExpr and
+      this.getMethodName() = "header"
     }
 
-    override RouteHandler getRouteHandler() { astNode.getReceiver() = result.getAResponseExpr() }
+    override RouteHandler getRouteHandler() {
+      this.getReceiver().asExpr() = result.getAResponseExpr()
+    }
   }
 
   /**

javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionCustomizations.qll

@@ -57,11 +57,11 @@ module RemotePropertyInjection {
    * header names as properties. This case is already handled by
    * `PropertyWriteSink`.
    */
-  class HeaderNameSink extends Sink, DataFlow::ValueNode {
+  class HeaderNameSink extends Sink {
     HeaderNameSink() {
       exists(HTTP::ExplicitHeaderDefinition hd |
         not hd instanceof Express::SetMultipleHeaders and
-        astNode = hd.getNameExpr()
+        this = hd.getNameNode()
       )
     }