Skip to content

Commit 555d7e5

Browse files
erik-krogherik-krogh
authored
Merge pull request github#14293 from am0o0/amammad-js-CodeInjection_dynamic_import
JS: Dynamic import as code injection sink
2 parents 60ed517 + bb03a9f commit 555d7e5

File tree

7 files changed

+231
-0
lines changed

7 files changed

+231
-0
lines changed

‎javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.inc.qhelp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Directly evaluating user input (for example, an HTTP request parameter) as code without properly
9+
sanitizing the input first allows an attacker arbitrary code execution. This can occur when user
10+
input is treated as JavaScript, or passed to a framework which interprets it as an expression to be
11+
evaluated. Examples include AngularJS expressions or JQuery selectors.
12+
</p>
13+
</overview>
14+
15+
<recommendation>
16+
<p>
17+
Avoid including user input in any expression which may be dynamically evaluated. If user input must
18+
be included, use context-specific escaping before
19+
including it. It is important that the correct escaping is used for the type of evaluation that will
20+
occur.
21+
</p>
22+
</recommendation>
23+
24+
<example>
25+
<p>
26+
The following example shows part of the page URL being evaluated as JavaScript code on the server. This allows an
27+
attacker to provide JavaScript within the URL and send it to server. client side attacks need victim users interaction
28+
like clicking on a attacker provided URL.
29+
</p>
30+
31+
<sample src="examples/CodeInjection.js" />
32+
33+
</example>
34+
35+
<references>
36+
<li>
37+
OWASP:
38+
<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
39+
</li>
40+
<li>
41+
Wikipedia: <a href="https://en.wikipedia.org/wiki/Code_injection">Code Injection</a>.
42+
</li>
43+
<li>
44+
PortSwigger Research Blog:
45+
<a href="https://portswigger.net/research/server-side-template-injection">Server-Side Template Injection</a>.
46+
</li>
47+
</references>
48+
</qhelp>

‎javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.qhelp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<include src="CodeInjection.inc.qhelp" />
6+
</qhelp>

‎javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* @name Code injection
3+
* @description Interpreting unsanitized user input as code allows a malicious user arbitrary
4+
* code execution.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 9.3
8+
* @precision high
9+
* @id js/code-injection-dynamic-import
10+
* @tags security
11+
* external/cwe/cwe-094
12+
* external/cwe/cwe-095
13+
* external/cwe/cwe-079
14+
* external/cwe/cwe-116
15+
*/
16+
17+
import javascript
18+
import DataFlow
19+
import DataFlow::PathGraph
20+
21+
abstract class Sanitizer extends DataFlow::Node { }
22+
23+
/** A non-first leaf in a string-concatenation. Seen as a sanitizer for dynamic import code injection. */
24+
class NonFirstStringConcatLeaf extends Sanitizer {
25+
NonFirstStringConcatLeaf() {
26+
exists(StringOps::ConcatenationRoot root |
27+
this = root.getALeaf() and
28+
not this = root.getFirstLeaf()
29+
)
30+
or
31+
exists(DataFlow::CallNode join |
32+
join = DataFlow::moduleMember("path", "join").getACall() and
33+
this = join.getArgument([1 .. join.getNumArgument() - 1])
34+
)
35+
}
36+
}
37+
38+
/**
39+
* The dynamic import expression input can be a `data:` URL which loads any module from that data
40+
*/
41+
class DynamicImport extends DataFlow::ExprNode {
42+
DynamicImport() { this = any(DynamicImportExpr e).getSource().flow() }
43+
}
44+
45+
/**
46+
* The dynamic import expression input can be a `data:` URL which loads any module from that data
47+
*/
48+
class WorkerThreads extends DataFlow::Node {
49+
WorkerThreads() {
50+
this = API::moduleImport("node:worker_threads").getMember("Worker").getParameter(0).asSink()
51+
}
52+
}
53+
54+
class UrlConstructorLabel extends FlowLabel {
55+
UrlConstructorLabel() { this = "UrlConstructorLabel" }
56+
}
57+
58+
/**
59+
* A taint-tracking configuration for reasoning about code injection vulnerabilities.
60+
*/
61+
class Configuration extends TaintTracking::Configuration {
62+
Configuration() { this = "CodeInjection" }
63+
64+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
65+
66+
override predicate isSink(DataFlow::Node sink) { sink instanceof DynamicImport }
67+
68+
override predicate isSink(DataFlow::Node sink, FlowLabel label) {
69+
sink instanceof WorkerThreads and label instanceof UrlConstructorLabel
70+
}
71+
72+
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
73+
74+
override predicate isAdditionalFlowStep(
75+
DataFlow::Node pred, DataFlow::Node succ, FlowLabel predlbl, FlowLabel succlbl
76+
) {
77+
exists(DataFlow::NewNode newUrl | succ = newUrl |
78+
newUrl = DataFlow::globalVarRef("URL").getAnInstantiation() and
79+
pred = newUrl.getArgument(0)
80+
) and
81+
predlbl instanceof StandardFlowLabel and
82+
succlbl instanceof UrlConstructorLabel
83+
}
84+
}
85+
86+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
87+
where cfg.hasFlowPath(source, sink)
88+
select sink.getNode(), source, sink, "This command line depends on a $@.", source.getNode(),
89+
"user-provided value"

‎javascript/ql/src/experimental/Security/CWE-094-dataURL/examples/CodeInjection.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
const { Worker } = require('node:worker_threads');
2+
var app = require('express')();
3+
4+
app.post('/path', async function (req, res) {
5+
const payload = req.query.queryParameter // like: payload = 'data:text/javascript,console.log("hello!");//'
6+
const payloadURL = new URL(payload)
7+
new Worker(payloadURL);
8+
});
9+
10+
app.post('/path2', async function (req, res) {
11+
const payload = req.query.queryParameter // like: payload = 'data:text/javascript,console.log("hello!");//'
12+
await import(payload)
13+
});
14+

‎javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.expected

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
nodes
2+
| test.js:5:11:5:44 | payload |
3+
| test.js:5:21:5:44 | req.que ... rameter |
4+
| test.js:5:21:5:44 | req.que ... rameter |
5+
| test.js:6:9:6:43 | payloadURL |
6+
| test.js:6:22:6:43 | new URL ... + sth) |
7+
| test.js:6:30:6:36 | payload |
8+
| test.js:6:30:6:42 | payload + sth |
9+
| test.js:7:16:7:25 | payloadURL |
10+
| test.js:7:16:7:25 | payloadURL |
11+
| test.js:9:5:9:39 | payloadURL |
12+
| test.js:9:18:9:39 | new URL ... + sth) |
13+
| test.js:9:26:9:32 | payload |
14+
| test.js:9:26:9:38 | payload + sth |
15+
| test.js:10:16:10:25 | payloadURL |
16+
| test.js:10:16:10:25 | payloadURL |
17+
| test.js:17:11:17:44 | payload |
18+
| test.js:17:21:17:44 | req.que ... rameter |
19+
| test.js:17:21:17:44 | req.que ... rameter |
20+
| test.js:18:18:18:24 | payload |
21+
| test.js:18:18:18:24 | payload |
22+
| test.js:19:18:19:24 | payload |
23+
| test.js:19:18:19:30 | payload + sth |
24+
| test.js:19:18:19:30 | payload + sth |
25+
edges
26+
| test.js:5:11:5:44 | payload | test.js:6:30:6:36 | payload |
27+
| test.js:5:11:5:44 | payload | test.js:9:26:9:32 | payload |
28+
| test.js:5:21:5:44 | req.que ... rameter | test.js:5:11:5:44 | payload |
29+
| test.js:5:21:5:44 | req.que ... rameter | test.js:5:11:5:44 | payload |
30+
| test.js:6:9:6:43 | payloadURL | test.js:7:16:7:25 | payloadURL |
31+
| test.js:6:9:6:43 | payloadURL | test.js:7:16:7:25 | payloadURL |
32+
| test.js:6:22:6:43 | new URL ... + sth) | test.js:6:9:6:43 | payloadURL |
33+
| test.js:6:30:6:36 | payload | test.js:6:30:6:42 | payload + sth |
34+
| test.js:6:30:6:42 | payload + sth | test.js:6:22:6:43 | new URL ... + sth) |
35+
| test.js:9:5:9:39 | payloadURL | test.js:10:16:10:25 | payloadURL |
36+
| test.js:9:5:9:39 | payloadURL | test.js:10:16:10:25 | payloadURL |
37+
| test.js:9:18:9:39 | new URL ... + sth) | test.js:9:5:9:39 | payloadURL |
38+
| test.js:9:26:9:32 | payload | test.js:9:26:9:38 | payload + sth |
39+
| test.js:9:26:9:38 | payload + sth | test.js:9:18:9:39 | new URL ... + sth) |
40+
| test.js:17:11:17:44 | payload | test.js:18:18:18:24 | payload |
41+
| test.js:17:11:17:44 | payload | test.js:18:18:18:24 | payload |
42+
| test.js:17:11:17:44 | payload | test.js:19:18:19:24 | payload |
43+
| test.js:17:21:17:44 | req.que ... rameter | test.js:17:11:17:44 | payload |
44+
| test.js:17:21:17:44 | req.que ... rameter | test.js:17:11:17:44 | payload |
45+
| test.js:19:18:19:24 | payload | test.js:19:18:19:30 | payload + sth |
46+
| test.js:19:18:19:24 | payload | test.js:19:18:19:30 | payload + sth |
47+
#select
48+
| 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 |
49+
| 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 |
50+
| 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 |
51+
| 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 |

‎javascript/ql/test/experimental/Security/CWE-094-dataURL/CodeInjection.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE-094-dataURL/CodeInjection.ql

‎javascript/ql/test/experimental/Security/CWE-094-dataURL/test.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
const { Worker } = require('node:worker_threads');
2+
var app = require('express')();
3+
4+
app.post('/path', async function (req, res) {
5+
const payload = req.query.queryParameter // like: payload = 'data:text/javascript,console.log("hello!");//'
6+
let payloadURL = new URL(payload + sth) // NOT OK
7+
new Worker(payloadURL);
8+
9+
payloadURL = new URL(payload + sth) // NOT OK
10+
new Worker(payloadURL);
11+
12+
payloadURL = new URL(sth + payload) // OK
13+
new Worker(payloadURL);
14+
});
15+
16+
app.post('/path2', async function (req, res) {
17+
const payload = req.query.queryParameter // like: payload = 'data:text/javascript,console.log("hello!");//'
18+
await import(payload) // NOT OK
19+
await import(payload + sth) // NOT OK
20+
await import(sth + payload) // OK
21+
});
22+

0 commit comments

Comments
 (0)