JS: improve Koa model to account for aliases on the context object

2026-04-30 03:05:15 +02:00 · 2019-03-21 15:03:09 +01:00
parent 0e01988622
commit 298dbe13c4
2 changed files with 64 additions and 31 deletions
--- a/javascript/ql/src/semmle/javascript/frameworks/Koa.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/Koa.qll
@@ -24,7 +24,7 @@ module Koa {

    HeaderDefinition() {
      // ctx.set('Cache-Control', 'no-cache');
-      astNode.calls(rh.getAContextExpr(), "set")
+      astNode.calls(rh.getAResponseOrContextExpr(), "set")
      or
      // ctx.response.header('Cache-Control', 'no-cache')
      astNode.calls(rh.getAResponseExpr(), "header")
@@ -58,6 +58,23 @@ module Koa {
     * route handler.
     */
    Expr getAContextExpr() { result.(ContextExpr).getRouteHandler() = this }
+
+    /**
+     * Gets an expression that contains the context or response
+     * object of a route handler invocation.
+     */
+    Expr getAResponseOrContextExpr() {
+      result = getAResponseExpr() or result = getAContextExpr()
+    }
+
+    /**
+     * Gets an expression that contains the context or request
+     * object of a route handler invocation.
+     */
+    Expr getARequestOrContextExpr() {
+      result = getARequestExpr() or result = getAContextExpr()
+    }
+
  }

  /**
@@ -159,35 +176,39 @@ module Koa {
    string kind;

    RequestInputAccess() {
-      exists(Expr request | request = rh.getARequestExpr() |
-        // `ctx.request.body`
-        kind = "body" and
-        this.asExpr().(PropAccess).accesses(request, "body")
-        or
-        kind = "parameter" and
-        this = getAQueryParameterAccess(rh)
-        or
+      kind = "parameter" and
+      this = getAQueryParameterAccess(rh)
+      or
+      exists(Expr e | rh.getARequestOrContextExpr() = e |
+        // `ctx.request.url`, `ctx.request.originalUrl`, or `ctx.request.href`
        exists(string propName |
-          // `ctx.request.url`, `ctx.request.originalUrl`, or `ctx.request.href`
          kind = "url" and
-          this.asExpr().(PropAccess).accesses(request, propName)
-        |
-          propName = "url" or
-          propName = "originalUrl" or
+          this.asExpr().(PropAccess).accesses(e, propName)
+          |
+          propName = "url"
+          or
+          propName = "originalUrl"
+          or
          propName = "href"
        )
-      )
-      or
-      exists(PropAccess cookies |
+        or
+        // `ctx.request.body`
+        e instanceof RequestExpr and
+        kind = "body" and
+        this.asExpr().(PropAccess).accesses(e, "body")
+        or
        // `ctx.cookies.get(<name>)`
-        kind = "cookie" and
-        cookies.accesses(rh.getAContextExpr(), "cookies") and
-        this.asExpr().(MethodCallExpr).calls(cookies, "get")
-      )
-      or
-      exists(RequestHeaderAccess access | access = this |
-        rh = access.getRouteHandler() and
-        kind = "header"
+        exists(PropAccess cookies |
+          e instanceof ContextExpr and
+          kind = "cookie" and
+          cookies.accesses(e, "cookies") and
+          this.asExpr().(MethodCallExpr).calls(cookies, "get")
+        )
+        or
+        exists(RequestHeaderAccess access | access = this |
+          rh = access.getRouteHandler() and
+          kind = "header"
+        )
      )
    }

@@ -199,8 +220,8 @@ module Koa {
  }

  private DataFlow::Node getAQueryParameterAccess(RouteHandler rh) {
-    // `ctx.request.query.name`
-    result.asExpr().(PropAccess).getBase().(PropAccess).accesses(rh.getARequestExpr(), "query")
+    // `ctx.query.name` or `ctx.request.query.name`
+    result.asExpr().(PropAccess).getBase().(PropAccess).accesses(rh.getARequestOrContextExpr(), "query")
  }

  /**
@@ -210,10 +231,10 @@ module Koa {
    RouteHandler rh;

    RequestHeaderAccess() {
-      exists(Expr request | request = rh.getARequestExpr() |
+      exists(Expr e | e = rh.getARequestOrContextExpr() |
        exists(string propName, PropAccess headers |
          // `ctx.request.header.<name>`, `ctx.request.headers.<name>`
-          headers.accesses(request, propName) and
+          headers.accesses(e, propName) and
          this.asExpr().(PropAccess).accesses(headers, _)
        |
          propName = "header" or
@@ -221,7 +242,7 @@ module Koa {
        )
        or
        // `ctx.request.get(<name>)`
-        this.asExpr().(MethodCallExpr).calls(request, "get")
+        this.asExpr().(MethodCallExpr).calls(e, "get")
      )
    }

@@ -264,7 +285,7 @@ module Koa {

    ResponseSendArgument() {
      exists(DataFlow::PropWrite pwn |
-        pwn.writes(DataFlow::valueNode(rh.getAResponseExpr()), "body", DataFlow::valueNode(this))
+        pwn.writes(DataFlow::valueNode(rh.getAResponseOrContextExpr()), "body", DataFlow::valueNode(this))
      )
    }

--- a/javascript/ql/test/library-tests/frameworks/koa/tests.expected
+++ b/javascript/ql/test/library-tests/frameworks/koa/tests.expected
@@ -12,6 +12,14 @@ test_RequestInputAccess
 | src/koa.js:25:3:25:25 | ctx.req ... ers.bar | header | src/koa.js:10:10:28:1 | functio ... az');\\n} |
 | src/koa.js:26:3:26:24 | ctx.req ... ('bar') | header | src/koa.js:10:10:28:1 | functio ... az');\\n} |
 | src/koa.js:27:3:27:24 | ctx.coo ... ('baz') | cookie | src/koa.js:10:10:28:1 | functio ... az');\\n} |
+| src/koa.js:33:2:33:14 | ctx.query.foo | parameter | src/koa.js:30:10:45:1 | async c ... url);\\n} |
+| src/koa.js:34:2:34:8 | ctx.url | url | src/koa.js:30:10:45:1 | async c ... url);\\n} |
+| src/koa.js:35:2:35:16 | ctx.originalUrl | url | src/koa.js:30:10:45:1 | async c ... url);\\n} |
+| src/koa.js:36:2:36:9 | ctx.href | url | src/koa.js:30:10:45:1 | async c ... url);\\n} |
+| src/koa.js:37:2:37:15 | ctx.header.bar | header | src/koa.js:30:10:45:1 | async c ... url);\\n} |
+| src/koa.js:38:2:38:16 | ctx.headers.bar | header | src/koa.js:30:10:45:1 | async c ... url);\\n} |
+| src/koa.js:40:2:40:15 | ctx.get('bar') | header | src/koa.js:30:10:45:1 | async c ... url);\\n} |
+| src/koa.js:42:12:42:27 | ctx.query.target | parameter | src/koa.js:30:10:45:1 | async c ... url);\\n} |
 test_RouteHandler_getAResponseHeader
 | src/koa.js:10:10:28:1 | functio ... az');\\n} | header1 | src/koa.js:11:3:11:25 | this.se ... 1', '') |
 | src/koa.js:10:10:28:1 | functio ... az');\\n} | header2 | src/koa.js:12:3:12:37 | this.re ... 2', '') |
@@ -81,6 +89,9 @@ test_HeaderAccess
 | src/koa.js:24:3:24:24 | ctx.req ... der.bar | bar |
 | src/koa.js:25:3:25:25 | ctx.req ... ers.bar | bar |
 | src/koa.js:26:3:26:24 | ctx.req ... ('bar') | bar |
+| src/koa.js:37:2:37:15 | ctx.header.bar | bar |
+| src/koa.js:38:2:38:16 | ctx.headers.bar | bar |
+| src/koa.js:40:2:40:15 | ctx.get('bar') | bar |
 test_RouteHandler_getAResponseExpr
 | src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:12:3:12:15 | this.response |
 | src/koa.js:10:10:28:1 | functio ... az');\\n} | src/koa.js:14:3:14:14 | ctx.response |
@@ -90,6 +101,7 @@ test_RouteHandler_getAResponseExpr
 | src/koa.js:30:10:45:1 | async c ... url);\\n} | src/koa.js:44:2:44:13 | ctx.response |
 test_ResponseSendArgument
 | src/koa.js:18:23:18:23 | x | src/koa.js:10:10:28:1 | functio ... az');\\n} |
+| src/koa.js:31:13:31:13 | x | src/koa.js:30:10:45:1 | async c ... url);\\n} |
 test_RouteSetup_getARouteHandler
 | src/koa.js:8:1:8:18 | app2.use(handler1) | src/koa.js:7:1:7:22 | functio ... r1() {} |
 | src/koa.js:10:1:28:2 | app2.us ... z');\\n}) | src/koa.js:10:10:28:1 | functio ... az');\\n} |