Merge branch 'master' into denisl/js/HttpToFileAccessTest

This commit is contained in:
Denis Levin
2018-10-02 15:13:35 -07:00
committed by GitHub
44 changed files with 746 additions and 96 deletions

View File

@@ -33,12 +33,15 @@ predicate deadStoreOfLocal(VarDef vd, PurelyLocalVariable v) {
* is itself an expression evaluating to `null` or `undefined`.
*/
predicate isNullOrUndef(Expr e) {
// `null` or `undefined`
e instanceof NullLiteral or
e.(VarAccess).getName() = "undefined" or
e instanceof VoidExpr or
// recursive case to catch multi-assignments of the form `x = y = null`
isNullOrUndef(e.(AssignExpr).getRhs())
exists (Expr inner |
inner = e.stripParens() |
// `null` or `undefined`
inner instanceof NullLiteral or
inner.(VarAccess).getName() = "undefined" or
inner instanceof VoidExpr or
// recursive case to catch multi-assignments of the form `x = y = null`
isNullOrUndef(inner.(AssignExpr).getRhs())
)
}
/**

View File

@@ -39,7 +39,7 @@ property of the name stored in variable <code>member</code>:
<p>
However, this test is ineffective as written: the operator <code>!</code> binds more
tighly than <code>in</code>, so it is applied first. Applying <code>!</code> to a
tightly than <code>in</code>, so it is applied first. Applying <code>!</code> to a
non-empty string yields <code>false</code>, so the <code>in</code> operator actually
ends up checking whether <code>obj</code> contains a property called <code>"false"</code>.
</p>

View File

@@ -0,0 +1,46 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Using the HTTP Host header to construct a link in an email can facilitate phishing attacks and leak password reset tokens.
A malicious user can send an HTTP request to the targeted web site, but with a Host header that refers to his own web site.
This means the emails will be sent out to potential victims, originating from a server they trust, but with
links leading to a malicious web site.
</p>
<p>
If the email contains a password reset link, and should the victim click the link, the secret reset token will be leaked to the attacker.
Using the leaked token, the attacker can then construct the real reset link and use it to change the victim's password.
</p>
</overview>
<recommendation>
<p>
Obtain the server's host name from a configuration file and avoid relying on the Host header.
</p>
</recommendation>
<example>
<p>
The following example uses the <code>req.host</code> to generate a password reset link.
This value is derived from the Host header, and can thus be set to anything by an attacker:
</p>
<sample src="examples/HostHeaderPoisoningInEmailGeneration.js"/>
<p>
To ensure the link refers to the correct web site, get the host name from a configuration file:
</p>
<sample src="examples/HostHeaderPoisoningInEmailGenerationGood.js"/>
</example>
<references>
<li>
Mitre:
<a href="https://cwe.mitre.org/data/definitions/640.html">CWE-640: Weak Password Recovery Mechanism for Forgotten Password</a>.
</li>
<li>
Ian Muscat:
<a href="https://www.acunetix.com/blog/articles/automated-detection-of-host-header-attacks/">What is a Host Header Attack?</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,29 @@
/**
* @name Host header poisoning in email generation
* @description Using the HTTP Host header to construct a link in an email can facilitate phishing attacks and leak password reset tokens.
* @kind problem
* @problem.severity error
* @precision high
* @id js/host-header-forgery-in-email-generation
* @tags security
* external/cwe/cwe-640
*/
import javascript
class TaintedHostHeader extends TaintTracking::Configuration {
TaintedHostHeader() { this = "TaintedHostHeader" }
override predicate isSource(DataFlow::Node node) {
exists (HTTP::RequestHeaderAccess input | node = input |
input.getKind() = "header" and
input.getAHeaderName() = "host")
}
override predicate isSink(DataFlow::Node node) {
exists (EmailSender email | node = email.getABody())
}
}
from TaintedHostHeader taint, DataFlow::Node src, DataFlow::Node sink
where taint.hasFlow(src, sink)
select sink, "Links in this email can be hijacked by poisoning the HTTP host header $@.", src, "here"

View File

@@ -0,0 +1,19 @@
let nodemailer = require('nodemailer');
let express = require('express');
let backend = require('./backend');
let app = express();
let config = JSON.parse(fs.readFileSync('config.json', 'utf8'));
app.post('/resetpass', (req, res) => {
let email = req.query.email;
let transport = nodemailer.createTransport(config.smtp);
let token = backend.getUserSecretResetToken(email);
transport.sendMail({
from: 'webmaster@example.com',
to: email,
subject: 'Forgot password',
text: `Click to reset password: https://${req.host}/resettoken/${token}`,
});
});

View File

@@ -0,0 +1,19 @@
let nodemailer = require('nodemailer');
let express = require('express');
let backend = require('./backend');
let app = express();
let config = JSON.parse(fs.readFileSync('config.json', 'utf8'));
app.post('/resetpass', (req, res) => {
let email = req.query.email;
let transport = nodemailer.createTransport(config.smtp);
let token = backend.getUserSecretResetToken(email);
transport.sendMail({
from: 'webmaster@example.com',
to: email,
subject: 'Forgot password',
text: `Click to reset password: https://${config.hostname}/resettoken/${token}`,
});
});

View File

@@ -13,6 +13,7 @@ import semmle.javascript.Constants
import semmle.javascript.DataFlow
import semmle.javascript.DefUse
import semmle.javascript.DOM
import semmle.javascript.EmailClients
import semmle.javascript.Errors
import semmle.javascript.ES2015Modules
import semmle.javascript.Expr

View File

@@ -0,0 +1,68 @@
import javascript
/**
* An operation that sends an email.
*/
abstract class EmailSender extends DataFlow::DefaultSourceNode {
/**
* Gets a data flow node holding the plaintext version of the email body.
*/
abstract DataFlow::Node getPlainTextBody();
/**
* Gets a data flow node holding the HTML body of the email.
*/
abstract DataFlow::Node getHtmlBody();
/**
* Gets a data flow node holding the address of the email recipient(s).
*/
abstract DataFlow::Node getTo();
/**
* Gets a data flow node holding the address of the email sender.
*/
abstract DataFlow::Node getFrom();
/**
* Gets a data flow node holding the email subject.
*/
abstract DataFlow::Node getSubject();
/**
* Gets a data flow node that refers to the HTML body or plaintext body of the email.
*/
DataFlow::Node getABody() {
result = getPlainTextBody() or
result = getHtmlBody()
}
}
/**
* An email-sending call based on the `nodemailer` package.
*/
private class NodemailerEmailSender extends EmailSender, DataFlow::MethodCallNode {
NodemailerEmailSender() {
this = DataFlow::moduleMember("nodemailer", "createTransport").getACall().getAMethodCall("sendMail")
}
override DataFlow::Node getPlainTextBody() {
result = getOptionArgument(0, "text")
}
override DataFlow::Node getHtmlBody() {
result = getOptionArgument(0, "html")
}
override DataFlow::Node getTo() {
result = getOptionArgument(0, "to")
}
override DataFlow::Node getFrom() {
result = getOptionArgument(0, "from")
}
override DataFlow::Node getSubject() {
result = getOptionArgument(0, "subject")
}
}

View File

@@ -283,6 +283,18 @@ abstract class AdditionalSink extends DataFlow::Node {
abstract predicate isSinkFor(Configuration cfg);
}
/**
* An invocation that is modeled as a partial function application.
*
* This contributes additional argument-passing flow edges that should be added to all data flow configurations.
*/
cached abstract class AdditionalPartialInvokeNode extends DataFlow::InvokeNode {
/**
* Holds if `argument` is passed as argument `index` to the function in `callback`.
*/
cached abstract predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index);
}
/**
* Additional flow step to model flow from import specifiers into the SSA variable
* corresponding to the imported variable.
@@ -299,6 +311,37 @@ private class FlowStepThroughImport extends AdditionalFlowStep, DataFlow::ValueN
}
}
/**
* A partial call through the built-in `Function.prototype.bind`.
*/
private class BindPartialCall extends AdditionalPartialInvokeNode, DataFlow::MethodCallNode {
BindPartialCall() {
getMethodName() = "bind"
}
override predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index) {
callback = getReceiver() and
argument = getArgument(index + 1)
}
}
/**
* A partial call through `_.partial` or a function with a similar interface.
*/
private class LibraryPartialCall extends AdditionalPartialInvokeNode {
LibraryPartialCall() {
this = LodashUnderscore::member("partial").getACall() or
this = DataFlow::moduleMember("ramda", "partial").getACall()
}
override predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index) {
callback = getArgument(0) and
exists (DataFlow::ArrayLiteralNode array |
array.flowsTo(getArgument(1)) and
argument = array.getElement(index))
}
}
/**
* Holds if there is a flow step from `pred` to `succ` described by `summary`
* under configuration `cfg`.
@@ -395,16 +438,18 @@ private predicate isRelevant(DataFlow::Node nd, DataFlow::Configuration cfg) {
* either `pred` is an argument of `f` and `succ` the corresponding parameter, or
* `pred` is a variable definition whose value is captured by `f` at `succ`.
*/
pragma[noopt]
private predicate callInputStep(Function f, DataFlow::Node invk,
DataFlow::Node pred, DataFlow::Node succ,
DataFlow::Configuration cfg) {
isRelevant(pred, cfg) and
(
isRelevant(pred, cfg) and
exists (Parameter parm |
argumentPassing(invk, pred, f, parm) and
succ = DataFlow::parameterNode(parm)
)
or
isRelevant(pred, cfg) and
exists (SsaDefinition prevDef, SsaDefinition def |
pred = DataFlow::ssaDefinitionNode(prevDef) and
calls(invk, f) and captures(f, prevDef, def) and
@@ -445,6 +490,7 @@ private predicate flowThroughCall(DataFlow::Node input, DataFlow::Node invk,
DataFlow::Configuration cfg, boolean valuePreserving) {
exists (Function f, DataFlow::ValueNode ret |
ret.asExpr() = f.getAReturnedExpr() and
calls(invk, f) and // Do not consider partial calls
reachableFromInput(f, invk, input, ret, cfg, PathSummary::level(valuePreserving))
)
}

View File

@@ -27,6 +27,21 @@ predicate calls(DataFlow::InvokeNode invk, Function f) {
f = invk.getACallee()
}
/**
* Holds if `invk` may invoke `f` indirectly through the given `callback` argument.
*
* This only holds for explicitly modeled partial calls.
*/
private predicate partiallyCalls(DataFlow::AdditionalPartialInvokeNode invk, DataFlow::AnalyzedNode callback, Function f) {
invk.isPartialArgument(callback, _, _) and
exists (AbstractFunction callee | callee = callback.getAValue() |
if callback.getAValue().isIndefinite("global") then
(f = callee.getFunction() and f.getFile() = invk.getFile())
else
f = callee.getFunction()
)
}
/**
* Holds if `f` captures the variable defined by `def` in `cap`.
*/
@@ -69,6 +84,11 @@ predicate argumentPassing(DataFlow::InvokeNode invk, DataFlow::ValueNode arg, Fu
f.getParameter(i) = parm and not parm.isRestParameter() and
arg = invk.getArgument(i)
)
or
exists (DataFlow::Node callback, int i |
invk.(DataFlow::AdditionalPartialInvokeNode).isPartialArgument(callback, arg, i) and
partiallyCalls(invk, callback, f) and
parm = f.getParameter(i) and not parm.isRestParameter())
}

View File

@@ -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() {
@@ -505,6 +490,53 @@ module Express {
}
}
/**
* 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() {
exists (string name |
name = this.(DataFlow::PropRead).getPropertyName()
or
this.(DataFlow::CallNode).getArgument(0).mayHaveStringValue(name)
|
if name = "hostname" then
result = "host"
else
result = name.toLowerCase())
}
override RouteHandler getRouteHandler() {
result = rh
}
override string getKind() {
result = "header"
}
}
/**
* HTTP headers created by Express calls
*/
@@ -600,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))
@@ -618,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()
)
)
}
}

View File

@@ -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();
}
@@ -359,9 +359,9 @@ module HTTP {
headerName = getNameExpr().getStringValue().toLowerCase() and
headerValue = astNode.getArgument(1)
}
override Expr getNameExpr() {
result = astNode.getArgument(0)
result = astNode.getArgument(0)
}
}
@@ -405,7 +405,20 @@ module HTTP {
*/
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 name of the header is unknown, this has no result.
*/
abstract string getAHeaderName();
}
/**
* A node that looks like a route setup on a server.
*

View File

@@ -121,13 +121,6 @@ module Hapi {
this.asExpr().(PropAccess).accesses(url, "path")
)
or
exists (PropAccess headers |
// `request.headers.<name>`
kind = "header" and
headers.accesses(request, "headers") and
this.asExpr().(PropAccess).accesses(headers, _)
)
or
exists (PropAccess state |
// `request.state.<name>`
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() {
@@ -146,6 +143,35 @@ module Hapi {
}
}
/**
* 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.<name>`
headers.accesses(request, "headers") and
this.asExpr().(PropAccess).accesses(headers, _)
)
)
}
override string getAHeaderName() {
result = this.(DataFlow::PropRead).getPropertyName().toLowerCase()
}
override RouteHandler getRouteHandler() {
result = rh
}
override string getKind() {
result = "header"
}
}
/**
* An HTTP header defined in a Hapi server.
*/

View File

@@ -182,19 +182,6 @@ module Koa {
propName = "originalUrl" or
propName = "href"
)
or
exists (string propName, PropAccess headers |
// `ctx.request.header.<name>`, `ctx.request.headers.<name>`
kind = "header" and
headers.accesses(request, propName) and
this.asExpr().(PropAccess).accesses(headers, _) |
propName = "header" or
propName = "headers"
)
or
// `ctx.request.get(<name>)`
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() {
@@ -214,6 +205,44 @@ module Koa {
}
}
/**
* 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.<name>`, `ctx.request.headers.<name>`
headers.accesses(request, propName) and
this.asExpr().(PropAccess).accesses(headers, _) |
propName = "header" or
propName = "headers"
)
or
// `ctx.request.get(<name>)`
this.asExpr().(MethodCallExpr).calls(request, "get")
)
}
override string getAHeaderName() {
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"
}
}
/**
* A call to a Koa method that sets up a route.
*/

View File

@@ -146,12 +146,16 @@ module NodeJSLib {
kind = "url" and
this.asExpr().(PropAccess).accesses(request, "url")
or
exists (PropAccess headers, string name |
// `req.headers.<name>`
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() {
@@ -163,6 +167,38 @@ module NodeJSLib {
}
}
/**
* 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>`
name != "cookie" and
headers.accesses(request, "headers") and
this.asExpr().(PropAccess).accesses(headers, name)
)
}
override string getAHeaderName() {
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 {
ServerDefinition server;
Expr handler;
@@ -537,7 +573,7 @@ module NodeJSLib {
}
}
/**
* A call to a method from module `child_process`.
*/
@@ -633,21 +669,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
@@ -700,7 +736,7 @@ module NodeJSLib {
}
}
/**
* A model of a URL request in the Node.js `http` library.
*/
@@ -726,7 +762,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) => {})`.
*/
@@ -736,12 +772,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) => {})})`.
*/
@@ -753,7 +789,7 @@ module NodeJSLib {
this = mcn.getCallback(1).getParameter(0)
)
}
override string getSourceType() {
result = "http.request data parameter"
}
@@ -771,13 +807,13 @@ module NodeJSLib {
}
}
/**
/**
* 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
@@ -785,14 +821,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.
*/
@@ -800,7 +836,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) => {})`.
*/
@@ -811,12 +847,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) => {})})`.
*/
@@ -828,12 +864,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.
*/
@@ -842,7 +878,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) => {})`.
*/
@@ -852,12 +888,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) => {})`.
*/
@@ -868,7 +904,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)})`.
*/
@@ -878,14 +914,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() {
@@ -893,13 +929,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) => {})`.
*/
@@ -910,7 +946,7 @@ module NodeJSLib {
handler.getAHandledEvent() = "error"
)
}
override string getSourceType() {
result = "NodeJSClientRequest error event"
}

View File

@@ -21,10 +21,10 @@ module ServerSideUrlRedirect {
* Holds if this sink may redirect to a non-local URL.
*/
predicate maybeNonLocal() {
exists (Expr prefix | prefix = getAPrefix(this) |
not exists(prefix.getStringValue())
exists (DataFlow::Node prefix | prefix = getAPrefix(this) |
not exists(prefix.asExpr().getStringValue())
or
exists (string prefixVal | prefixVal = prefix.getStringValue() |
exists (string prefixVal | prefixVal = prefix.asExpr().getStringValue() |
// local URLs (i.e., URLs that start with `/` not followed by `\` or `/`,
// or that start with `~/`) are unproblematic
not prefixVal.regexpMatch("/[^\\\\/].*|~/.*") and
@@ -47,12 +47,12 @@ module ServerSideUrlRedirect {
/**
* Gets an expression that may end up being a prefix of the string concatenation `nd`.
*/
private Expr getAPrefix(Sink sink) {
private DataFlow::Node getAPrefix(Sink sink) {
exists (DataFlow::Node prefix |
prefix = prefixCandidate(sink) and
not exists(StringConcatenation::getFirstOperand(prefix)) and
not exists(prefix.getAPredecessor()) and
result = prefix.asExpr()
result = prefix
)
}

View File

@@ -0,0 +1 @@
| tst.js:17:2:19:3 | transpo ... ');\\n\\t}) | tst.js:11:12:11:31 | 'sender@example.com' | tst.js:12:10:12:55 | 'receiv ... le.com' | tst.js:13:15:13:28 | 'Some subject' | tst.js:14:12:14:15 | 'Hi' | tst.js:15:12:15:22 | '<b>Hi</b>' |

View File

@@ -0,0 +1,4 @@
import javascript
from EmailSender send
select send, send.getFrom(), send.getTo(), send.getSubject(), send.getPlainTextBody(), send.getHtmlBody()

View File

@@ -0,0 +1,20 @@
let nodemailer = require('nodemailer');
let config = require('./account-config');
function sendMessage() {
let transporter = nodemailer.createTransport({
host: config.host,
port: config.host,
auth: config.auth
});
let mailOptions = {
from: 'sender@example.com',
to: 'receiver1@example.com, receiver2@example.com',
subject: 'Some subject',
text: 'Hi',
html: '<b>Hi</b>'
};
transporter.sendMail(mailOptions, (error, info) => {
console.log('Message sent');
});
}

View File

@@ -1,3 +1,8 @@
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:17:14:17:14 | x |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:20:14:20:14 | y |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:41:10:41:18 | id(taint) |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:51:14:51:14 | x |
| tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |
| tst.js:2:13:2:20 | source() | tst.js:5:10:5:22 | "/" + x + "!" |
| tst.js:2:13:2:20 | source() | tst.js:14:10:14:17 | x.sort() |

View File

@@ -0,0 +1,58 @@
let R = require('ramda');
function test() {
let taint = source();
function safe1(x, y) {
sink(x); // OK - x is not tainted
}
function safe2(x, y) {
sink(y); // OK - y is not tainted
}
safe1.bind(null, "hello", taint)();
safe2.bind(null, taint, "hello")();
function unsafe1(x, y) {
sink(x); // NOT OK - x is tainted
}
function unsafe2(x ,y) {
sink(y); // NOT OK - y is tainted
}
unsafe1.bind(null, taint, "hello")();
unsafe2.bind(null, "hello", taint)();
function safeprop(x) {
sink(x.value); // OK - property `value` is not tainted
}
function unsafeprop(x) {
sink(x.value); // NOT OK - property `value` is tainted
}
safeprop.bind(null, {value: "hello", somethingElse: taint})();
unsafeprop.bind(null, {value: taint, somethingElse: "hello"})();
function id(x) {
return x;
}
sink(id("hello")); // OK
sink(id(taint)); // NOT OK
let taintGetter = id.bind(null, taint);
sink(taintGetter); // OK - this is a function object
sink(taintGetter()); // NOT OK - but not currently detected
function safearray(x) {
sink(x); // OK
}
function unsafearray(x) {
sink(x); // NOT OK
}
let xs = ["hello"];
let ys = [taint];
R.partial(safearray, xs)();
R.partial(unsafearray, ys)();
}

View File

@@ -0,0 +1,5 @@
| src/express.js:28:3:28:16 | req.get("foo") | foo |
| src/express.js:29:3:29:19 | req.header("bar") | bar |
| src/express.js:47:3:47:17 | req.headers.baz | baz |
| src/express.js:48:3:48:10 | req.host | host |
| src/express.js:49:3:49:14 | req.hostname | host |

View File

@@ -0,0 +1,4 @@
import javascript
from HTTP::RequestHeaderAccess access
select access, access.getAHeaderName()

View File

@@ -0,0 +1,2 @@
| src/http.js:9:3:9:17 | req.headers.foo | foo |
| src/https.js:9:3:9:17 | req.headers.foo | foo |

View File

@@ -0,0 +1,4 @@
import javascript
from HTTP::RequestHeaderAccess access
select access, access.getAHeaderName()

View File

@@ -0,0 +1 @@
| src/hapi.js:25:3:25:21 | request.headers.baz | baz |

View File

@@ -0,0 +1,4 @@
import javascript
from HTTP::RequestHeaderAccess access
select access, access.getAHeaderName()

View File

@@ -0,0 +1,3 @@
| 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 |

View File

@@ -0,0 +1,4 @@
import javascript
from HTTP::RequestHeaderAccess access
select access, access.getAHeaderName()

View File

@@ -141,4 +141,15 @@ function v() {
x = 23;
x = 42;
return x;
}
}
(function(){
var x = (void 0);
var y = ((void 0));
var z1 = z2 = (void 0);
x = 42;
y = 42;
z1 = 42;
z2 = 42;
return x + y + z1 + z2;
});

View File

@@ -2,6 +2,10 @@
| etherpad.js:11:12:11:19 | response | Cross-site scripting vulnerability due to $@. | etherpad.js:9:16:9:30 | req.query.jsonp | user-provided value |
| formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| partial.js:10:14:10:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:13:42:13:48 | req.url | user-provided value |
| partial.js:19:14:19:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:22:52:22:58 | req.url | user-provided value |
| partial.js:28:14:28:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:31:48:31:54 | req.url | user-provided value |
| partial.js:37:14:37:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:40:43:40:49 | req.url | user-provided value |
| promises.js:6:25:6:25 | x | Cross-site scripting vulnerability due to $@. | promises.js:5:44:5:57 | req.query.data | user-provided value |
| tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |
| tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value |

View File

@@ -11,6 +11,18 @@ edges
| formatting.js:4:16:4:29 | req.query.evil | formatting.js:4:9:4:29 | evil |
| formatting.js:6:43:6:46 | evil | formatting.js:6:14:6:47 | util.fo ... , evil) |
| formatting.js:7:49:7:52 | evil | formatting.js:7:14:7:53 | require ... , evil) |
| partial.js:9:25:9:25 | x | partial.js:10:14:10:14 | x |
| partial.js:10:14:10:14 | x | partial.js:10:14:10:18 | x + y |
| partial.js:13:42:13:48 | req.url | partial.js:9:25:9:25 | x |
| partial.js:18:25:18:25 | x | partial.js:19:14:19:14 | x |
| partial.js:19:14:19:14 | x | partial.js:19:14:19:18 | x + y |
| partial.js:22:52:22:58 | req.url | partial.js:18:25:18:25 | x |
| partial.js:27:25:27:25 | x | partial.js:28:14:28:14 | x |
| partial.js:28:14:28:14 | x | partial.js:28:14:28:18 | x + y |
| partial.js:31:48:31:54 | req.url | partial.js:27:25:27:25 | x |
| partial.js:36:25:36:25 | x | partial.js:37:14:37:14 | x |
| partial.js:37:14:37:14 | x | partial.js:37:14:37:18 | x + y |
| partial.js:40:43:40:49 | req.url | partial.js:36:25:36:25 | x |
| promises.js:5:3:5:59 | new Pro ... .data)) | promises.js:6:11:6:11 | x |
| promises.js:5:44:5:57 | req.query.data | promises.js:5:3:5:59 | new Pro ... .data)) |
| promises.js:5:44:5:57 | req.query.data | promises.js:6:11:6:11 | x |
@@ -25,6 +37,10 @@ edges
| etherpad.js:11:12:11:19 | response | etherpad.js:9:16:9:30 | req.query.jsonp | etherpad.js:11:12:11:19 | response | Cross-site scripting vulnerability due to $@. | etherpad.js:9:16:9:30 | req.query.jsonp | user-provided value |
| formatting.js:6:14:6:47 | util.fo ... , evil) | formatting.js:4:16:4:29 | req.query.evil | formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| formatting.js:7:14:7:53 | require ... , evil) | formatting.js:4:16:4:29 | req.query.evil | formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| partial.js:10:14:10:18 | x + y | partial.js:13:42:13:48 | req.url | partial.js:10:14:10:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:13:42:13:48 | req.url | user-provided value |
| partial.js:19:14:19:18 | x + y | partial.js:22:52:22:58 | req.url | partial.js:19:14:19:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:22:52:22:58 | req.url | user-provided value |
| partial.js:28:14:28:18 | x + y | partial.js:31:48:31:54 | req.url | partial.js:28:14:28:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:31:48:31:54 | req.url | user-provided value |
| partial.js:37:14:37:18 | x + y | partial.js:40:43:40:49 | req.url | partial.js:37:14:37:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:40:43:40:49 | req.url | user-provided value |
| promises.js:6:25:6:25 | x | promises.js:5:44:5:57 | req.query.data | promises.js:6:25:6:25 | x | Cross-site scripting vulnerability due to $@. | promises.js:5:44:5:57 | req.query.data | user-provided value |
| promises.js:6:25:6:25 | x | promises.js:5:44:5:57 | req.query.data | promises.js:6:25:6:25 | x | Cross-site scripting vulnerability due to $@. | promises.js:5:44:5:57 | req.query.data | user-provided value |
| tst2.js:7:12:7:12 | p | tst2.js:6:9:6:9 | p | tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |

View File

@@ -1,6 +1,10 @@
| ReflectedXss.js:8:14:8:45 | "Unknow ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:8:33:8:45 | req.params.id | user-provided value |
| formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| partial.js:10:14:10:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:13:42:13:48 | req.url | user-provided value |
| partial.js:19:14:19:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:22:52:22:58 | req.url | user-provided value |
| partial.js:28:14:28:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:31:48:31:54 | req.url | user-provided value |
| partial.js:37:14:37:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:40:43:40:49 | req.url | user-provided value |
| promises.js:6:25:6:25 | x | Cross-site scripting vulnerability due to $@. | promises.js:5:44:5:57 | req.query.data | user-provided value |
| tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |
| tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value |

View File

@@ -5,6 +5,10 @@ WARNING: Predicate flowsFrom has been deprecated and may be removed in future (R
| ReflectedXss.js:8:14:8:45 | "Unknow ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:8:33:8:45 | req.params.id | user-provided value |
| formatting.js:6:14:6:47 | util.fo ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| formatting.js:7:14:7:53 | require ... , evil) | Cross-site scripting vulnerability due to $@. | formatting.js:4:16:4:29 | req.query.evil | user-provided value |
| partial.js:10:14:10:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:13:42:13:48 | req.url | user-provided value |
| partial.js:19:14:19:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:22:52:22:58 | req.url | user-provided value |
| partial.js:28:14:28:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:31:48:31:54 | req.url | user-provided value |
| partial.js:37:14:37:18 | x + y | Cross-site scripting vulnerability due to $@. | partial.js:40:43:40:49 | req.url | user-provided value |
| promises.js:6:25:6:25 | x | Cross-site scripting vulnerability due to $@. | promises.js:5:44:5:57 | req.query.data | user-provided value |
| tst2.js:7:12:7:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:6:9:6:9 | p | user-provided value |
| tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value |

View File

@@ -0,0 +1,55 @@
let express = require('express');
let underscore = require('underscore');
let lodash = require('lodash');
let R = require('ramda');
let app = express();
app.get("/some/path", (req, res) => {
function sendResponse(x, y) {
res.send(x + y); // NOT OK
}
let callback = sendResponse.bind(null, req.url);
[1, 2, 3].forEach(callback);
});
app.get("/underscore", (req, res) => {
function sendResponse(x, y) {
res.send(x + y); // NOT OK
}
let callback = underscore.partial(sendResponse, [req.url]);
[1, 2, 3].forEach(callback);
});
app.get("/lodash", (req, res) => {
function sendResponse(x, y) {
res.send(x + y); // NOT OK
}
let callback = lodash.partial(sendResponse, [req.url]);
[1, 2, 3].forEach(callback);
});
app.get("/ramda", (req, res) => {
function sendResponse(x, y) {
res.send(x + y); // NOT OK
}
let callback = R.partial(sendResponse, [req.url]);
[1, 2, 3].forEach(callback);
});
app.get("/return", (req, res) => {
function getFirst(x, y) {
return x;
}
let callback = getFirst.bind(null, req.url);
res.send(callback); // OK - the callback itself is not tainted
res.send(callback()); // NOT OK - but not currently detected
res.send(getFirst("Hello")); // OK - argument is not tainted from this call site
});

View File

@@ -3,6 +3,7 @@
| express.js:33:18:33:23 | target | Untrusted URL redirection due to $@. | express.js:27:16:27:34 | req.param("target") | user-provided value |
| express.js:35:16:35:21 | target | Untrusted URL redirection due to $@. | express.js:27:16:27:34 | req.param("target") | user-provided value |
| express.js:44:16:44:108 | (req.pa ... ntacts" | Untrusted URL redirection due to $@. | express.js:44:69:44:87 | req.param('action') | user-provided value |
| express.js:53:26:53:28 | url | Untrusted URL redirection due to $@. | express.js:48:16:48:28 | req.params[0] | user-provided value |
| express.js:78:16:78:43 | `${req. ... )}/foo` | Untrusted URL redirection due to $@. | express.js:78:19:78:37 | req.param("target") | user-provided value |
| express.js:94:18:94:23 | target | Untrusted URL redirection due to $@. | express.js:87:16:87:34 | req.param("target") | user-provided value |
| express.js:101:16:101:21 | target | Untrusted URL redirection due to $@. | express.js:87:16:87:34 | req.param("target") | user-provided value |

View File

@@ -121,3 +121,12 @@ app.get('/array/join', function(req, res) {
// BAD: request input becomes before query string
res.redirect([req.query.page, '?section=', req.query.section].join(''));
});
function sendUserToUrl(res, nextUrl) {
// BAD: value comes from query parameter
res.redrect(nextUrl);
}
app.get('/call', function(req, res) {
sendUserToUrl(res, req.query.nextUrl);
});

View File

@@ -0,0 +1,2 @@
| tst.js:17:11:17:113 | `Hi, lo ... token}` | Links in this email can be hijacked by poisoning the HTTP host header $@. | tst.js:17:84:17:91 | req.host | here |
| tst.js:18:11:18:127 | `Hi, lo ... reset.` | Links in this email can be hijacked by poisoning the HTTP host header $@. | tst.js:18:78:18:85 | req.host | here |

View File

@@ -0,0 +1 @@
Security/CWE-640/HostHeaderPoisoningInEmailGeneration.ql

View File

@@ -0,0 +1,28 @@
let nodemailer = require('nodemailer');
let express = require('express');
let app = express();
let backend = require('./backend');
app.post('/resetpass', (req, res) => {
let email = req.query.email;
let transport = nodemailer.createTransport({});
let token = backend.getUserSecretResetToken(email);
transport.sendMail({
from: 'webmaster@example.com',
to: email,
subject: 'Forgot password',
text: `Hi, looks like you forgot your password. Click here to reset: https://${req.host}/resettoken/${token}`, // NOT OK
html: `Hi, looks like you forgot your password. Click <a href="https://${req.host}/resettoken/${token}">here</a> to reset.` // NOT OK
});
transport.sendMail({
from: 'webmaster@example.com',
to: email,
subject: 'Forgot password',
text: `Hi, looks like you forgot your password. Click here to reset: https://example.com/resettoken/${token}`, // OK
html: `Hi, looks like you forgot your password. Click <a href="https://example.com/resettoken/${token}">here</a> to reset.` // OK
});
});