Merge pull request #14293 from am0o0/amammad-js-CodeInjection_dynamic_import

JS: Dynamic import as code injection sink
2026-04-25 00:35:20 +02:00 · 2024-06-20 21:19:57 +02:00
parent 60ed51781e bb03a9faba
commit 555d7e5958
7 changed files with 231 additions and 0 deletions
--- a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.inc.qhelp
+++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.inc.qhelp
@@ -0,0 +1,48 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+Directly evaluating user input (for example, an HTTP request parameter) as code without properly
+sanitizing the input first allows an attacker arbitrary code execution. This can occur when user
+input is treated as JavaScript, or passed to a framework which interprets it as an expression to be
+evaluated. Examples include AngularJS expressions or JQuery selectors.
+</p>
+</overview>
+
+<recommendation>
+<p>
+Avoid including user input in any expression which may be dynamically evaluated. If user input must
+be included, use context-specific escaping before
+including it. It is important that the correct escaping is used for the type of evaluation that will
+occur.
+</p>
+</recommendation>
+
+<example>
+<p>
+The following example shows part of the page URL being evaluated as JavaScript code on the server. This allows an
+attacker to provide JavaScript within the URL and send it to server. client side attacks need victim users interaction 
+like clicking on a attacker provided URL.
+</p>
+
+<sample src="examples/CodeInjection.js" />
+
+</example>
+
+<references>
+<li>
+OWASP:
+<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
+</li>
+<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>
--- a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.qhelp
+++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.qhelp
@@ -0,0 +1,6 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+<include src="CodeInjection.inc.qhelp" />
+</qhelp>
--- a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql
+++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql
@@ -0,0 +1,89 @@
+/**
+ * @name Code injection
+ * @description Interpreting unsanitized user input as code allows a malicious user arbitrary
+ *              code execution.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 9.3
+ * @precision high
+ * @id js/code-injection-dynamic-import
+ * @tags security
+ *       external/cwe/cwe-094
+ *       external/cwe/cwe-095
+ *       external/cwe/cwe-079
+ *       external/cwe/cwe-116
+ */
+
+import javascript
+import DataFlow
+import DataFlow::PathGraph
+
+abstract class Sanitizer extends DataFlow::Node { }
+
+/** A non-first leaf in a string-concatenation. Seen as a sanitizer for dynamic import code injection. */
+class NonFirstStringConcatLeaf extends Sanitizer {
+  NonFirstStringConcatLeaf() {
+    exists(StringOps::ConcatenationRoot root |
+      this = root.getALeaf() and
+      not this = root.getFirstLeaf()
+    )
+    or
+    exists(DataFlow::CallNode join |
+      join = DataFlow::moduleMember("path", "join").getACall() and
+      this = join.getArgument([1 .. join.getNumArgument() - 1])
+    )
+  }
+}
+
+/**
+ * The dynamic import expression input can be a `data:` URL which loads any module from that data
+ */
+class DynamicImport extends DataFlow::ExprNode {
+  DynamicImport() { this = any(DynamicImportExpr e).getSource().flow() }
+}
+
+/**
+ * The dynamic import expression input can be a `data:` URL which loads any module from that data
+ */
+class WorkerThreads extends DataFlow::Node {
+  WorkerThreads() {
+    this = API::moduleImport("node:worker_threads").getMember("Worker").getParameter(0).asSink()
+  }
+}
+
+class UrlConstructorLabel extends FlowLabel {
+  UrlConstructorLabel() { this = "UrlConstructorLabel" }
+}
+
+/**
+ * A taint-tracking configuration for reasoning about code injection vulnerabilities.
+ */
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "CodeInjection" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof DynamicImport }
+
+  override predicate isSink(DataFlow::Node sink, FlowLabel label) {
+    sink instanceof WorkerThreads and label instanceof UrlConstructorLabel
+  }
+
+  override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+
+  override predicate isAdditionalFlowStep(
+    DataFlow::Node pred, DataFlow::Node succ, FlowLabel predlbl, FlowLabel succlbl
+  ) {
+    exists(DataFlow::NewNode newUrl | succ = newUrl |
+      newUrl = DataFlow::globalVarRef("URL").getAnInstantiation() and
+      pred = newUrl.getArgument(0)
+    ) and
+    predlbl instanceof StandardFlowLabel and
+    succlbl instanceof UrlConstructorLabel
+  }
+}
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "This command line depends on a $@.", source.getNode(),
+  "user-provided value"
--- a/javascript/ql/src/experimental/Security/CWE-094-dataURL/examples/CodeInjection.js
+++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/examples/CodeInjection.js
@@ -0,0 +1,14 @@
+const { Worker } = require('node:worker_threads');
+var app = require('express')();
+
+app.post('/path', async function (req, res) {
+    const payload = req.query.queryParameter // like:  payload = 'data:text/javascript,console.log("hello!");//'
+    const payloadURL = new URL(payload)
+    new Worker(payloadURL);
+});
+
+app.post('/path2', async function (req, res) {
+    const payload = req.query.queryParameter // like:  payload = 'data:text/javascript,console.log("hello!");//'
+    await import(payload)
+});
+
--- a/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.expected
+++ b/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.expected
@@ -0,0 +1,51 @@
+nodes
+| test.js:5:11:5:44 | payload |
+| test.js:5:21:5:44 | req.que ... rameter |
+| test.js:5:21:5:44 | req.que ... rameter |
+| test.js:6:9:6:43 | payloadURL |
+| test.js:6:22:6:43 | new URL ...  + sth) |
+| test.js:6:30:6:36 | payload |
+| test.js:6:30:6:42 | payload + sth |
+| test.js:7:16:7:25 | payloadURL |
+| test.js:7:16:7:25 | payloadURL |
+| test.js:9:5:9:39 | payloadURL |
+| test.js:9:18:9:39 | new URL ...  + sth) |
+| test.js:9:26:9:32 | payload |
+| test.js:9:26:9:38 | payload + sth |
+| test.js:10:16:10:25 | payloadURL |
+| test.js:10:16:10:25 | payloadURL |
+| test.js:17:11:17:44 | payload |
+| test.js:17:21:17:44 | req.que ... rameter |
+| test.js:17:21:17:44 | req.que ... rameter |
+| test.js:18:18:18:24 | payload |
+| test.js:18:18:18:24 | payload |
+| test.js:19:18:19:24 | payload |
+| test.js:19:18:19:30 | payload + sth |
+| test.js:19:18:19:30 | payload + sth |
+edges
+| test.js:5:11:5:44 | payload | test.js:6:30:6:36 | payload |
+| test.js:5:11:5:44 | payload | test.js:9:26:9:32 | payload |
+| test.js:5:21:5:44 | req.que ... rameter | test.js:5:11:5:44 | payload |
+| test.js:5:21:5:44 | req.que ... rameter | test.js:5:11:5:44 | payload |
+| test.js:6:9:6:43 | payloadURL | test.js:7:16:7:25 | payloadURL |
+| test.js:6:9:6:43 | payloadURL | test.js:7:16:7:25 | payloadURL |
+| test.js:6:22:6:43 | new URL ...  + sth) | test.js:6:9:6:43 | payloadURL |
+| test.js:6:30:6:36 | payload | test.js:6:30:6:42 | payload + sth |
+| test.js:6:30:6:42 | payload + sth | test.js:6:22:6:43 | new URL ...  + sth) |
+| test.js:9:5:9:39 | payloadURL | test.js:10:16:10:25 | payloadURL |
+| test.js:9:5:9:39 | payloadURL | test.js:10:16:10:25 | payloadURL |
+| test.js:9:18:9:39 | new URL ...  + sth) | test.js:9:5:9:39 | payloadURL |
+| test.js:9:26:9:32 | payload | test.js:9:26:9:38 | payload + sth |
+| test.js:9:26:9:38 | payload + sth | test.js:9:18:9:39 | new URL ...  + sth) |
+| test.js:17:11:17:44 | payload | test.js:18:18:18:24 | payload |
+| test.js:17:11:17:44 | payload | test.js:18:18:18:24 | payload |
+| test.js:17:11:17:44 | payload | test.js:19:18:19:24 | payload |
+| test.js:17:21:17:44 | req.que ... rameter | test.js:17:11:17:44 | payload |
+| test.js:17:21:17:44 | req.que ... rameter | test.js:17:11:17:44 | payload |
+| test.js:19:18:19:24 | payload | test.js:19:18:19:30 | payload + sth |
+| test.js:19:18:19:24 | payload | test.js:19:18:19:30 | payload + sth |
+#select
+| test.js:7:16:7:25 | payloadURL | test.js:5:21:5:44 | req.que ... rameter | test.js:7:16:7:25 | payloadURL | This command line depends on a $@. | test.js:5:21:5:44 | req.que ... rameter | user-provided value |
+| test.js:10:16:10:25 | payloadURL | test.js:5:21:5:44 | req.que ... rameter | test.js:10:16:10:25 | payloadURL | This command line depends on a $@. | test.js:5:21:5:44 | req.que ... rameter | user-provided value |
+| test.js:18:18:18:24 | payload | test.js:17:21:17:44 | req.que ... rameter | test.js:18:18:18:24 | payload | This command line depends on a $@. | test.js:17:21:17:44 | req.que ... rameter | user-provided value |
+| test.js:19:18:19:30 | payload + sth | test.js:17:21:17:44 | req.que ... rameter | test.js:19:18:19:30 | payload + sth | This command line depends on a $@. | test.js:17:21:17:44 | req.que ... rameter | user-provided value |
--- a/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.qlref
+++ b/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-094-dataURL/CodeInjection.ql
--- a/javascript/ql/test/experimental/Security/CWE-094-dataURL/test.js
+++ b/javascript/ql/test/experimental/Security/CWE-094-dataURL/test.js
@@ -0,0 +1,22 @@
+const { Worker } = require('node:worker_threads');
+var app = require('express')();
+
+app.post('/path', async function (req, res) {
+    const payload = req.query.queryParameter // like:  payload = 'data:text/javascript,console.log("hello!");//'
+    let payloadURL = new URL(payload + sth) // NOT OK
+    new Worker(payloadURL);
+
+    payloadURL = new URL(payload + sth) // NOT OK
+    new Worker(payloadURL);
+
+    payloadURL = new URL(sth + payload) // OK
+    new Worker(payloadURL);
+});
+
+app.post('/path2', async function (req, res) {
+    const payload = req.query.queryParameter // like:  payload = 'data:text/javascript,console.log("hello!");//'
+    await import(payload) // NOT OK
+    await import(payload + sth) // NOT OK
+    await import(sth + payload) // OK
+});
+
				`@@ -0,0 +1 @@`
				`experimental/Security/CWE-094-dataURL/CodeInjection.ql`