Merge pull request #241 from asger-semmle/host-header-forgery

JS: Add HostHeaderPoisoningInEmailGeneration query

Author: Max Schaefer

change-notes/1.19/analysis-javascript.md

@@ -17,6 +17,7 @@
 | Enabling Node.js integration for Electron web content renderers (`js/enabling-electron-renderer-node-integration`) | security, frameworks/electron, external/cwe/cwe-094  | Highlights Electron web content renderer preferences with Node.js integration enabled, indicating a violation of [CWE-94](https://cwe.mitre.org/data/definitions/94.html). Results are not shown on LGTM by default. |
 | Stored cross-site scripting (`js/stored-xss`) | security, external/cwe/cwe-079, external/cwe/cwe-116 | Highlights uncontrolled stored values flowing into HTML content, indicating a violation of [CWE-079](https://cwe.mitre.org/data/definitions/79.html). Results shown on LGTM by default. |
 | Replacement of a substring with itself (`js/identity-replacement`) | correctness, security, external/cwe/cwe-116 | Highlights string replacements that replace a string with itself, which usually indicates a mistake. Results shown on LGTM by default. |
+| Host header poisoning in email generation |  security, external/cwe/cwe-640 | Highlights code that generates emails with links that can be hijacked by HTTP host header poisoning, indicating a violation of [CWE-640](https://cwe.mitre.org/data/definitions/640.html). Results shown on LGTM by default.  |
 
 ## Changes to existing queries
 

javascript/config/suites/javascript/security

@@ -22,6 +22,7 @@
 + semmlecode-javascript-queries/Security/CWE-601/ClientSideUrlRedirect.ql: /Security/CWE/CWE-601
 + semmlecode-javascript-queries/Security/CWE-601/ServerSideUrlRedirect.ql: /Security/CWE/CWE-601
 + semmlecode-javascript-queries/Security/CWE-611/Xxe.ql: /Security/CWE/CWE-611
++ semmlecode-javascript-queries/Security/CWE-640/HostHeaderPoisoningInEmailGeneration.ql: /Security/CWE/CWE-640
 + semmlecode-javascript-queries/Security/CWE-643/XpathInjection.ql: /Security/CWE/CWE-643
 + semmlecode-javascript-queries/Security/CWE-730/RegExpInjection.ql: /Security/CWE/CWE-730
 + semmlecode-javascript-queries/Security/CWE-770/MissingRateLimiting.ql: /Security/CWE/CWE-770

javascript/ql/src/Security/CWE-640/HostHeaderPoisoningInEmailGeneration.qhelp

@@ -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>

javascript/ql/src/Security/CWE-640/HostHeaderPoisoningInEmailGeneration.ql

@@ -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"

javascript/ql/src/Security/CWE-640/examples/HostHeaderPoisoningInEmailGeneration.js

@@ -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}`,
+  });
+});

javascript/ql/src/Security/CWE-640/examples/HostHeaderPoisoningInEmailGenerationGood.js

@@ -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}`,
+  });
+});

javascript/ql/src/javascript.qll

@@ -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

javascript/ql/src/semmle/javascript/EmailClients.qll

@@ -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")
+  }
+}

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

@@ -471,37 +471,69 @@ module Express {
           propName = "originalUrl"
         )
         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() {
+      result = rh
+    }
+
+    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(...)`
-          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")
       )
     }
 
+    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 = kind
+      result = "header"
     }
   }
 
@@ -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()
-      )      
+      )
     }
   }
 

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)
       }
 
     }
@@ -400,7 +400,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.
    *