diff --git a/javascript/ql/src/Security/CWE-640/HostHeaderPoisoningInEmailGeneration.ql b/javascript/ql/src/Security/CWE-640/HostHeaderPoisoningInEmailGeneration.ql index 134faf0f77d..9d653efb2f7 100644 --- a/javascript/ql/src/Security/CWE-640/HostHeaderPoisoningInEmailGeneration.ql +++ b/javascript/ql/src/Security/CWE-640/HostHeaderPoisoningInEmailGeneration.ql @@ -14,7 +14,7 @@ class TaintedHostHeader extends TaintTracking::Configuration { TaintedHostHeader() { this = "TaintedHostHeader" } override predicate isSource(DataFlow::Node node) { - exists (HTTP::RequestInputAccess input | node = input | + exists (HTTP::RequestHeaderAccess input | node = input | input.getKind() = "header" and input.getAHeaderName() = "host") } diff --git a/javascript/ql/src/semmle/javascript/frameworks/Express.qll b/javascript/ql/src/semmle/javascript/frameworks/Express.qll index 641058c061e..832b0550a84 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Express.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Express.qll @@ -471,29 +471,14 @@ module Express { propName = "originalUrl" ) or - exists (string methodName | - // `req.get(...)` or `req.header(...)` - kind = "header" and - this.(DataFlow::MethodCallNode).calls(request, methodName) | - methodName = "get" or - methodName = "header" - ) - or - exists (DataFlow::PropRead headers | - // `req.headers.name` - kind = "header" and - headers.accesses(request, "headers") and - this = headers.getAPropertyRead()) - or - exists (string propName | propName = "host" or propName = "hostname" | - // `req.host` and `req.hostname` are derived from headers - kind = "header" and - this.(DataFlow::PropRead).accesses(request, propName)) - or // `req.cookies` kind = "cookie" and this.(DataFlow::PropRef).accesses(request, "cookies") ) + or + exists (RequestHeaderAccess access | this = access | + rh = access.getRouteHandler() and + kind = "header") } override RouteHandler getRouteHandler() { @@ -503,9 +488,35 @@ module Express { override string getKind() { result = kind } + } + + /** + * An access to a header on an Express request. + */ + private class RequestHeaderAccess extends HTTP::RequestHeaderAccess { + RouteHandler rh; + + RequestHeaderAccess() { + exists (DataFlow::Node request | request = DataFlow::valueNode(rh.getARequestExpr()) | + exists (string methodName | + // `req.get(...)` or `req.header(...)` + this.(DataFlow::MethodCallNode).calls(request, methodName) | + methodName = "get" or + methodName = "header" + ) + or + exists (DataFlow::PropRead headers | + // `req.headers.name` + headers.accesses(request, "headers") and + this = headers.getAPropertyRead()) + or + exists (string propName | propName = "host" or propName = "hostname" | + // `req.host` and `req.hostname` are derived from headers + this.(DataFlow::PropRead).accesses(request, propName)) + ) + } override string getAHeaderName() { - kind = "header" and exists (string name | name = this.(DataFlow::PropRead).getPropertyName() or @@ -516,6 +527,14 @@ module Express { else result = name.toLowerCase()) } + + override RouteHandler getRouteHandler() { + result = rh + } + + override string getKind() { + result = "header" + } } /** @@ -613,9 +632,9 @@ module Express { astNode.getMethodName() = any(string n | n = "set" or n = "header") and astNode.getNumArgument() = 1 } - + /** - * Gets a reference to the multiple headers object that is to be set. + * Gets a reference to the multiple headers object that is to be set. */ private DataFlow::SourceNode getAHeaderSource() { result.flowsToExpr(astNode.getArgument(0)) @@ -631,12 +650,12 @@ module Express { override RouteHandler getRouteHandler() { result = rh } - + override Expr getNameExpr() { - exists (DataFlow::PropWrite write | + exists (DataFlow::PropWrite write | getAHeaderSource().flowsTo(write.getBase()) and result = write.getPropertyNameExpr() - ) + ) } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll b/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll index 2247613575a..55e617176c0 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/HTTP.qll @@ -72,9 +72,9 @@ 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); - + /** - * Returns the expression used to compute the header name. + * Returns the expression used to compute the header name. */ abstract Expr getNameExpr(); } @@ -354,9 +354,9 @@ module HTTP { headerName = getNameExpr().getStringValue().toLowerCase() and headerValue = astNode.getArgument(1) } - + override Expr getNameExpr() { - result = astNode.getArgument(0) + result = astNode.getArgument(0) } } @@ -399,15 +399,19 @@ module HTTP { * Note that this predicate is functional. */ abstract string getKind(); + } + /** + * An access to a header on an incoming HTTP request. + */ + abstract class RequestHeaderAccess extends RequestInputAccess { /** * Gets the lower-case name of an HTTP header from which this input is derived, * if this can be determined. * - * When the input is not derived from a header, or the header name is - * unknown, this has no result. + * When the name of the header is unknown, this has no result. */ - string getAHeaderName() { none() } + abstract string getAHeaderName(); } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll b/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll index b578521e41b..920d3c7496e 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Hapi.qll @@ -121,13 +121,6 @@ module Hapi { this.asExpr().(PropAccess).accesses(url, "path") ) or - exists (PropAccess headers | - // `request.headers.` - kind = "header" and - headers.accesses(request, "headers") and - this.asExpr().(PropAccess).accesses(headers, _) - ) - or exists (PropAccess state | // `request.state.` kind = "cookie" and @@ -135,6 +128,10 @@ module Hapi { this.asExpr().(PropAccess).accesses(state, _) ) ) + or + exists (RequestHeaderAccess access | this = access | + rh = access.getRouteHandler() and + kind = "header") } override RouteHandler getRouteHandler() { @@ -144,11 +141,35 @@ module Hapi { override string getKind() { result = kind } + } + + /** + * An access to an HTTP header on a Hapi request. + */ + private class RequestHeaderAccess extends HTTP::RequestHeaderAccess { + RouteHandler rh; + + RequestHeaderAccess() { + exists (Expr request | request = rh.getARequestExpr() | + exists (PropAccess headers | + // `request.headers.` + headers.accesses(request, "headers") and + this.asExpr().(PropAccess).accesses(headers, _) + ) + ) + } override string getAHeaderName() { - kind = "header" and result = this.(DataFlow::PropRead).getPropertyName().toLowerCase() } + + override RouteHandler getRouteHandler() { + result = rh + } + + override string getKind() { + result = "header" + } } /** diff --git a/javascript/ql/src/semmle/javascript/frameworks/Koa.qll b/javascript/ql/src/semmle/javascript/frameworks/Koa.qll index 22f2a37e8ca..6bf4979aff0 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/Koa.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/Koa.qll @@ -182,19 +182,6 @@ module Koa { propName = "originalUrl" or propName = "href" ) - or - exists (string propName, PropAccess headers | - // `ctx.request.header.`, `ctx.request.headers.` - kind = "header" and - headers.accesses(request, propName) and - this.asExpr().(PropAccess).accesses(headers, _) | - propName = "header" or - propName = "headers" - ) - or - // `ctx.request.get()` - kind = "header" and - this.asExpr().(MethodCallExpr).calls(request, "get") ) or exists (PropAccess cookies | @@ -203,6 +190,10 @@ module Koa { cookies.accesses(rh.getAContextExpr(), "cookies") and this.asExpr().(MethodCallExpr).calls(cookies, "get") ) + or + exists (RequestHeaderAccess access | access = this | + rh = access.getRouteHandler() and + kind = "header") } override RouteHandler getRouteHandler() { @@ -212,16 +203,43 @@ module Koa { override string getKind() { result = kind } + } + + /** + * An access to an HTTP header on a Koa request. + */ + private class RequestHeaderAccess extends HTTP::RequestHeaderAccess { + RouteHandler rh; + + RequestHeaderAccess() { + exists (Expr request | request = rh.getARequestExpr() | + exists (string propName, PropAccess headers | + // `ctx.request.header.`, `ctx.request.headers.` + headers.accesses(request, propName) and + this.asExpr().(PropAccess).accesses(headers, _) | + propName = "header" or + propName = "headers" + ) + or + // `ctx.request.get()` + this.asExpr().(MethodCallExpr).calls(request, "get") + ) + } override string getAHeaderName() { - kind = "header" and - ( - result = this.(DataFlow::PropRead).getPropertyName().toLowerCase() - or - exists (string name | - this.(DataFlow::CallNode).getArgument(0).mayHaveStringValue(name) and - result = name.toLowerCase()) - ) + result = this.(DataFlow::PropRead).getPropertyName().toLowerCase() + or + exists (string name | + this.(DataFlow::CallNode).getArgument(0).mayHaveStringValue(name) and + result = name.toLowerCase()) + } + + override RouteHandler getRouteHandler() { + result = rh + } + + override string getKind() { + result = "header" } } diff --git a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll index ab36e0f4a94..d604afaf1c3 100644 --- a/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll +++ b/javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll @@ -146,12 +146,16 @@ module NodeJSLib { kind = "url" and this.asExpr().(PropAccess).accesses(request, "url") or - exists (PropAccess headers, string name | - // `req.headers.` - if name = "cookie" then kind = "cookie" else kind= "header" | + exists (PropAccess headers | + // `req.headers.cookie` + kind = "cookie" and headers.accesses(request, "headers") and - this.asExpr().(PropAccess).accesses(headers, name) + this.asExpr().(PropAccess).accesses(headers, "cookie") ) + or + exists (RequestHeaderAccess access | this = access | + request = access.getRequest() and + kind = "header") } override RouteHandler getRouteHandler() { @@ -161,11 +165,38 @@ module NodeJSLib { override string getKind() { result = kind } + } + + /** + * An access to an HTTP header (other than "Cookie") on an incoming Node.js request object. + */ + private class RequestHeaderAccess extends HTTP::RequestHeaderAccess { + RequestExpr request; + + RequestHeaderAccess() { + exists (PropAccess headers, string name | + // `req.headers.` + name != "cookie" and + headers.accesses(request, "headers") and + this.asExpr().(PropAccess).accesses(headers, name) + ) + } override string getAHeaderName() { - kind = "header" and result = this.(DataFlow::PropRead).getPropertyName().toLowerCase() } + + override RouteHandler getRouteHandler() { + result = request.getRouteHandler() + } + + override string getKind() { + result = "header" + } + + RequestExpr getRequest() { + result = request + } } class RouteSetup extends CallExpr, HTTP::Servers::StandardRouteSetup { @@ -385,7 +416,7 @@ module NodeJSLib { } } - + /** * A call to a method from module `child_process`. */ @@ -481,21 +512,21 @@ module NodeJSLib { } } - + /** * A call to a method from module `vm` */ class VmModuleMethodCall extends DataFlow::CallNode { string methodName; - + VmModuleMethodCall() { this = DataFlow::moduleMember("vm", methodName).getACall() } - + /** * Gets the code to be executed as part of this call. */ - DataFlow::Node getACodeArgument() { + DataFlow::Node getACodeArgument() { ( methodName = "runInContext" or methodName = "runInNewContext" or @@ -548,7 +579,7 @@ module NodeJSLib { } } - + /** * A model of a URL request in the Node.js `http` library. */ @@ -574,7 +605,7 @@ module NodeJSLib { } } - + /** * A data flow node that is the parameter of a result callback for an HTTP or HTTPS request made by a Node.js process, for example `res` in `https.request(url, (res) => {})`. */ @@ -584,12 +615,12 @@ module NodeJSLib { this = req.(DataFlow::MethodCallNode).getCallback(1).getParameter(0) ) } - + override string getSourceType() { result = "NodeJSClientRequest callback parameter" } } - + /** * A data flow node that is the parameter of a data callback for an HTTP or HTTPS request made by a Node.js process, for example `body` in `http.request(url, (res) => {res.on('data', (body) => {})})`. */ @@ -601,20 +632,20 @@ module NodeJSLib { this = mcn.getCallback(1).getParameter(0) ) } - + override string getSourceType() { result = "http.request data parameter" } } - - + + /** * A data flow node that is registered as a callback for an HTTP or HTTPS request made by a Node.js process, for example the function `handler` in `http.request(url).on(message, handler)`. */ class ClientRequestHandler extends DataFlow::FunctionNode { string handledEvent; NodeJSClientRequest clientRequest; - + ClientRequestHandler() { exists(DataFlow::MethodCallNode mcn | clientRequest.getAMethodCall("on") = mcn and @@ -622,14 +653,14 @@ module NodeJSLib { flowsTo(mcn.getArgument(1)) ) } - + /** * Gets the name of an event this callback is registered for. */ string getAHandledEvent() { result = handledEvent } - + /** * Gets a request this callback is registered for. */ @@ -637,7 +668,7 @@ module NodeJSLib { result = clientRequest } } - + /** * A data flow node that is the parameter of a response callback for an HTTP or HTTPS request made by a Node.js process, for example `res` in `http.request(url).on('response', (res) => {})`. */ @@ -648,12 +679,12 @@ module NodeJSLib { handler.getAHandledEvent() = "response" ) } - + override string getSourceType() { result = "NodeJSClientRequest response event" } } - + /** * A data flow node that is the parameter of a data callback for an HTTP or HTTPS request made by a Node.js process, for example `chunk` in `http.request(url).on('response', (res) => {res.on('data', (chunk) => {})})`. */ @@ -665,12 +696,12 @@ module NodeJSLib { this = mcn.getCallback(1).getParameter(0) ) } - + override string getSourceType() { result = "NodeJSClientRequest data event" } } - + /** * A data flow node that is a login callback for an HTTP or HTTPS request made by a Node.js process. */ @@ -679,7 +710,7 @@ module NodeJSLib { getAHandledEvent() = "login" } } - + /** * A data flow node that is a parameter of a login callback for an HTTP or HTTPS request made by a Node.js process, for example `res` in `http.request(url).on('login', (res, callback) => {})`. */ @@ -689,12 +720,12 @@ module NodeJSLib { this = handler.getParameter(0) ) } - + override string getSourceType() { result = "NodeJSClientRequest login event" } } - + /** * A data flow node that is the login callback provided by an HTTP or HTTPS request made by a Node.js process, for example `callback` in `http.request(url).on('login', (res, callback) => {})`. */ @@ -705,7 +736,7 @@ module NodeJSLib { ) } } - + /** * A data flow node that is the username passed to the login callback provided by an HTTP or HTTPS request made by a Node.js process, for example `username` in `http.request(url).on('login', (res, cb) => {cb(username, password)})`. */ @@ -715,14 +746,14 @@ module NodeJSLib { this = callback.getACall().getArgument(0).asExpr() ) } - + override string getCredentialsKind() { result = "Node.js http(s) client login username" } } - + /** - * A data flow node that is the password passed to the login callback provided by an HTTP or HTTPS request made by a Node.js process, for example `password` in `http.request(url).on('login', (res, cb) => {cb(username, password)})`. + * A data flow node that is the password passed to the login callback provided by an HTTP or HTTPS request made by a Node.js process, for example `password` in `http.request(url).on('login', (res, cb) => {cb(username, password)})`. */ private class ClientRequestLoginPassword extends CredentialsExpr { ClientRequestLoginPassword() { @@ -730,13 +761,13 @@ module NodeJSLib { this = callback.getACall().getArgument(1).asExpr() ) } - + override string getCredentialsKind() { result = "Node.js http(s) client login password" } } - + /** * A data flow node that is the parameter of an error callback for an HTTP or HTTPS request made by a Node.js process, for example `err` in `http.request(url).on('error', (err) => {})`. */ @@ -747,7 +778,7 @@ module NodeJSLib { handler.getAHandledEvent() = "error" ) } - + override string getSourceType() { result = "NodeJSClientRequest error event" } diff --git a/javascript/ql/test/library-tests/frameworks/Express/HeaderAccess.ql b/javascript/ql/test/library-tests/frameworks/Express/HeaderAccess.ql index 74b64ef8a64..a86349b65cb 100644 --- a/javascript/ql/test/library-tests/frameworks/Express/HeaderAccess.ql +++ b/javascript/ql/test/library-tests/frameworks/Express/HeaderAccess.ql @@ -1,4 +1,4 @@ import javascript -from HTTP::RequestInputAccess access +from HTTP::RequestHeaderAccess access select access, access.getAHeaderName() diff --git a/javascript/ql/test/library-tests/frameworks/NodeJSLib/HeaderAccess.ql b/javascript/ql/test/library-tests/frameworks/NodeJSLib/HeaderAccess.ql index 74b64ef8a64..a86349b65cb 100644 --- a/javascript/ql/test/library-tests/frameworks/NodeJSLib/HeaderAccess.ql +++ b/javascript/ql/test/library-tests/frameworks/NodeJSLib/HeaderAccess.ql @@ -1,4 +1,4 @@ import javascript -from HTTP::RequestInputAccess access +from HTTP::RequestHeaderAccess access select access, access.getAHeaderName() diff --git a/javascript/ql/test/library-tests/frameworks/hapi/HeaderAccess.ql b/javascript/ql/test/library-tests/frameworks/hapi/HeaderAccess.ql index 74b64ef8a64..a86349b65cb 100644 --- a/javascript/ql/test/library-tests/frameworks/hapi/HeaderAccess.ql +++ b/javascript/ql/test/library-tests/frameworks/hapi/HeaderAccess.ql @@ -1,4 +1,4 @@ import javascript -from HTTP::RequestInputAccess access +from HTTP::RequestHeaderAccess access select access, access.getAHeaderName() diff --git a/javascript/ql/test/library-tests/frameworks/koa/HeaderAccess.ql b/javascript/ql/test/library-tests/frameworks/koa/HeaderAccess.ql index 74b64ef8a64..a86349b65cb 100644 --- a/javascript/ql/test/library-tests/frameworks/koa/HeaderAccess.ql +++ b/javascript/ql/test/library-tests/frameworks/koa/HeaderAccess.ql @@ -1,4 +1,4 @@ import javascript -from HTTP::RequestInputAccess access +from HTTP::RequestHeaderAccess access select access, access.getAHeaderName()