Merge pull request #4762 from asgerf/js/template-sinks-in-code-injection

Approved by erik-krogh, mchammer01

Author: codeql-ci

javascript/ql/src/Security/CWE-094/CodeInjection.qhelp

@@ -30,6 +30,21 @@ for example, steal cookies containing session information.
 </p>
 
 <sample src="examples/CodeInjection.js" />
+
+<p>
+The following example shows a Pug template being constructed from user input, allowing attackers to run
+arbitrary code via a payload such as <code>#{global.process.exit(1)}</code>.
+</p>
+
+<sample src="examples/ServerSideTemplateInjection.js" />
+
+<p>
+Below is an example of how to use a template engine without any risk of template injection.
+The user input is included via an interpolation expression <code>#{username}</code> whose value is provided
+as an option to the template, instead of being part of the template string itself:
+</p>
+
+<sample src="examples/ServerSideTemplateInjectionSafe.js" />
 </example>
 
 <references>
@@ -40,5 +55,9 @@ OWASP:
 <li>
 Wikipedia: <a href="https://en.wikipedia.org/wiki/Code_injection">Code Injection</a>.
 </li>
+<li>
+PortSwigger Research Blog:
+<a href="https://portswigger.net/research/server-side-template-injection">Server-Side Template Injection</a>.
+</li>
 </references>
 </qhelp>

javascript/ql/src/Security/CWE-094/CodeInjection.ql

@@ -18,5 +18,6 @@ import DataFlow::PathGraph
 
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
-select sink.getNode(), source, sink, "$@ flows to here and is interpreted as code.",
-  source.getNode(), "User-provided value"
+select sink.getNode(), source, sink,
+  "$@ flows to " + sink.getNode().(Sink).getMessageSuffix() + ".", source.getNode(),
+  "User-provided value"

javascript/ql/src/Security/CWE-094/examples/ServerSideTemplateInjection.js

@@ -0,0 +1,20 @@
+const express = require('express')
+var pug = require('pug');
+const app = express()
+
+app.post('/', (req, res) => {
+    var input = req.query.username;
+    var template = `
+doctype
+html
+head
+    title= 'Hello world'
+body
+    form(action='/' method='post')
+        input#name.form-control(type='text)
+        button.btn.btn-primary(type='submit') Submit
+    p Hello `+ input
+    var fn = pug.compile(template);
+    var html = fn();
+    res.send(html);
+})

javascript/ql/src/Security/CWE-094/examples/ServerSideTemplateInjectionSafe.js

@@ -0,0 +1,20 @@
+const express = require('express')
+var pug = require('pug');
+const app = express()
+
+app.post('/', (req, res) => {
+    var input = req.query.username;
+    var template = `
+doctype
+html
+head
+    title= 'Hello world'
+body
+    form(action='/' method='post')
+        input#name.form-control(type='text)
+        button.btn.btn-primary(type='submit') Submit
+    p Hello #{username}`
+    var fn = pug.compile(template);
+    var html = fn({username: input});
+    res.send(html);
+})

javascript/ql/src/experimental/Security/CWE-94/ServerSideTemplateInjection.qhelp

@@ -15,7 +15,12 @@ module CodeInjection {
   /**
    * A data flow sink for code injection vulnerabilities.
    */
-  abstract class Sink extends DataFlow::Node { }
+  abstract class Sink extends DataFlow::Node {
+    /**
+     * Gets the substitute for `X` in the message `User-provided value flows to X`.
+     */
+    string getMessageSuffix() { result = "here and is interpreted as code" }
+  }
 
   /**
    * A sanitizer for code injection vulnerabilities.
@@ -138,4 +143,57 @@ module CodeInjection {
         API::moduleImport("module").getInstance().getMember("_compile").getACall().getArgument(0)
     }
   }
+
+  /** A sink for code injection via template injection. */
+  abstract private class TemplateSink extends Sink {
+    override string getMessageSuffix() {
+      result = "here and is interpreted as a template, which may contain code"
+    }
+  }
+
+  /**
+   * A value interpreted as as template by the `pug` library.
+   */
+  class PugTemplateSink extends TemplateSink {
+    PugTemplateSink() {
+      this =
+        DataFlow::moduleImport(["pug", "jade"]).getAMemberCall(["compile", "render"]).getArgument(0)
+    }
+  }
+
+  /**
+   * A value interpreted as a tempalte by the `dot` library.
+   */
+  class DotTemplateSink extends TemplateSink {
+    DotTemplateSink() {
+      this = DataFlow::moduleImport("dot").getAMemberCall("template").getArgument(0)
+    }
+  }
+
+  /**
+   * A value interpreted as a template by the `ejs` library.
+   */
+  class EjsTemplateSink extends TemplateSink {
+    EjsTemplateSink() {
+      this = DataFlow::moduleImport("ejs").getAMemberCall("render").getArgument(0)
+    }
+  }
+
+  /**
+   * A value interpreted as a template by the `nunjucks` library.
+   */
+  class NunjucksTemplateSink extends TemplateSink {
+    NunjucksTemplateSink() {
+      this = DataFlow::moduleImport("nunjucks").getAMemberCall("renderString").getArgument(0)
+    }
+  }
+
+  /**
+   * A value interpreted as a template by `lodash` or `underscore`.
+   */
+  class LodashUnderscoreTemplateSink extends TemplateSink {
+    LodashUnderscoreTemplateSink() {
+      this = LodashUnderscore::member("template").getACall().getArgument(0)
+    }
+  }
 }