diff --git a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.inc.qhelp b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.inc.qhelp new file mode 100644 index 000000000000..39c02a629056 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.inc.qhelp @@ -0,0 +1,48 @@ + + + + +

+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. +

+ + + +

+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. +

+ + + +

+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. +

+ + + + + + +
  • +OWASP: +Code Injection. +
    • +
  • +Wikipedia: Code Injection. +
    • +
  • +PortSwigger Research Blog: +Server-Side Template Injection. +
    • +     +     diff --git a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.qhelp b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.qhelp new file mode 100644 index 000000000000..784368354731 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.qhelp @@ -0,0 +1,6 @@ + + + + \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql b/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql new file mode 100644 index 000000000000..b4b66293ee51 --- /dev/null +++ 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" diff --git a/javascript/ql/src/experimental/Security/CWE-094-dataURL/examples/CodeInjection.js b/javascript/ql/src/experimental/Security/CWE-094-dataURL/examples/CodeInjection.js new file mode 100644 index 000000000000..743b4e0d65d0 --- /dev/null +++ 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) +}); + diff --git a/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.expected b/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.expected new file mode 100644 index 000000000000..3a5963b4094f --- /dev/null +++ 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 | diff --git a/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.qlref b/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.qlref new file mode 100644 index 000000000000..3caf7ab7b43b --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-094-dataURL/CodeInjection.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094-dataURL/test.js b/javascript/ql/test/experimental/Security/CWE-094-dataURL/test.js new file mode 100644 index 000000000000..a5a2e76fa3c8 --- /dev/null +++ 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 +}); +