Skip to content

Commit 1fc481c

Browse files
committed
v2: it is basically the first stable version :))
1 parent 102f09a commit 1fc481c

File tree

14 files changed

+149
-16
lines changed

14 files changed

+149
-16
lines changed
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
8+
<p>
9+
Controlling the value of arbitrary environment variables from user-controllable data is not safe.
10+
</p>
11+
12+
</overview>
13+
14+
<recommendation>
15+
16+
<p>
17+
Restrict this operation only to privileged users or only for some not important environment variables.
18+
</p>
19+
20+
</recommendation>
21+
22+
<example>
23+
24+
<p>
25+
The following example allows unauthorized users to assign a value to any environment variable.
26+
</p>
27+
28+
<sample src="examples/Bad_Value_And_Key_Assignment" />
29+
30+
</example>
31+
32+
<references>
33+
<a href="https://huntr.com/bounties/00ec6847-125b-43e9-9658-d3cace1751d6/">Admin account TakeOver in mintplex-labs/anything-llm</a>
34+
</references>
35+
36+
</qhelp>

javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql renamed to javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
2-
* @name User controlled environment injection
3-
* @description full control on creating environment variables from user controlled data is not secure
2+
* @name User controlled arbitrary environment variable injection
3+
* @description creating arbitrary environment variables from user controlled data is not secure
44
* @kind path-problem
5-
* @id js/envinjection
5+
* @id js/env-key-and-value-injection
66
* @problem.severity error
77
* @security-severity 7.5
88
* @precision medium
@@ -46,19 +46,19 @@ class Configuration extends TaintTracking::Configuration {
4646
}
4747

4848
from
49-
Configuration cfg, Configuration cfg2, DataFlow::PathNode source, DataFlow::PathNode sink,
49+
Configuration cfg, Configuration cfg2, DataFlow::PathNode source, DataFlow::PathNode sink1,
5050
DataFlow::PathNode sink2
5151
where
52-
cfg.hasFlowPath(source, sink) and
53-
sink.getNode() = API::moduleImport("process").getMember("env").getAMember().asSink() and
52+
cfg.hasFlowPath(source, sink1) and
53+
sink1.getNode() = API::moduleImport("process").getMember("env").getAMember().asSink() and
5454
cfg2.hasFlowPath(source, sink2) and
55-
sink.getNode().asExpr() =
55+
sink2.getNode().asExpr() =
5656
NodeJSLib::process()
5757
.getAPropertyRead("env")
5858
.asExpr()
5959
.getParent()
6060
.(IndexExpr)
6161
.getAChildExpr()
6262
.(VarRef)
63-
select sink.getNode(), source, sink, "this environment variable assignment is $@.",
64-
source.getNode(), "user controllable"
63+
select sink1.getNode(), source, sink1, "arbitrary environment variable assignment from this $@.",
64+
source.getNode(), "user controllable source"

javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp renamed to javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
The following example allows unauthorized users to assign a value to a critical environment variable.
2626
</p>
2727

28-
<sample src="examples/Bad.js" />
28+
<sample src="examples/Bad_Value_Assignment.js" />
2929

3030
</example>
3131

javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql renamed to javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.ql

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
/**
2-
* @name User controlled environment injection
3-
* @description full control on creating environment variables from user controlled data is not secure
2+
* @name User controlled environment variable value injection
3+
* @description assigning important environment variables from user controlled data is not secure
44
* @kind path-problem
5-
* @id js/envinjection
5+
* @id js/env-value-injection
66
* @problem.severity error
77
* @security-severity 7.5
88
* @precision medium
@@ -24,7 +24,6 @@ class Configuration extends TaintTracking::Configuration {
2424
}
2525
}
2626

27-
2827
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
2928
where cfg.hasFlowPath(source, sink)
3029
select sink.getNode(), source, sink, "this environment variable assignment is $@.",
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
const http = require('node:http');
2+
3+
http.createServer((req, res) => {
4+
const { EnvValue, EnvKey } = req.body;
5+
process.env[EnvKey] = EnvValue; // NOT OK
6+
7+
res.end('env has been injected!');
8+
});

javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref

Lines changed: 0 additions & 1 deletion
This file was deleted.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
nodes
2+
| test.js:5:9:5:28 | { EnvValue, EnvKey } |
3+
| test.js:5:9:5:39 | EnvKey |
4+
| test.js:5:9:5:39 | EnvValue |
5+
| test.js:5:11:5:18 | EnvValue |
6+
| test.js:5:21:5:26 | EnvKey |
7+
| test.js:5:32:5:39 | req.body |
8+
| test.js:5:32:5:39 | req.body |
9+
| test.js:6:15:6:20 | EnvKey |
10+
| test.js:6:15:6:20 | EnvKey |
11+
| test.js:6:25:6:32 | EnvValue |
12+
| test.js:6:25:6:32 | EnvValue |
13+
| test.js:7:15:7:20 | EnvKey |
14+
| test.js:7:15:7:20 | EnvKey |
15+
| test.js:7:25:7:32 | EnvValue |
16+
| test.js:7:25:7:32 | EnvValue |
17+
| test.js:8:24:8:31 | EnvValue |
18+
| test.js:8:24:8:31 | EnvValue |
19+
edges
20+
| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:11:5:18 | EnvValue |
21+
| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:21:5:26 | EnvKey |
22+
| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey |
23+
| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey |
24+
| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey |
25+
| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey |
26+
| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue |
27+
| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue |
28+
| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue |
29+
| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue |
30+
| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue |
31+
| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue |
32+
| test.js:5:11:5:18 | EnvValue | test.js:5:9:5:39 | EnvValue |
33+
| test.js:5:21:5:26 | EnvKey | test.js:5:9:5:39 | EnvKey |
34+
| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } |
35+
| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } |
36+
#select
37+
| test.js:6:25:6:32 | EnvValue | test.js:5:32:5:39 | req.body | test.js:6:25:6:32 | EnvValue | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source |
38+
| test.js:7:25:7:32 | EnvValue | test.js:5:32:5:39 | req.body | test.js:7:25:7:32 | EnvValue | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source |
39+
| test.js:8:24:8:31 | EnvValue | test.js:5:32:5:39 | req.body | test.js:8:24:8:31 | EnvValue | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
nodes
2+
| test.js:5:9:5:28 | { EnvValue, EnvKey } |
3+
| test.js:5:9:5:39 | EnvKey |
4+
| test.js:5:9:5:39 | EnvValue |
5+
| test.js:5:11:5:18 | EnvValue |
6+
| test.js:5:21:5:26 | EnvKey |
7+
| test.js:5:32:5:39 | req.body |
8+
| test.js:5:32:5:39 | req.body |
9+
| test.js:6:15:6:20 | EnvKey |
10+
| test.js:6:15:6:20 | EnvKey |
11+
| test.js:6:25:6:32 | EnvValue |
12+
| test.js:6:25:6:32 | EnvValue |
13+
| test.js:7:15:7:20 | EnvKey |
14+
| test.js:7:15:7:20 | EnvKey |
15+
| test.js:7:25:7:32 | EnvValue |
16+
| test.js:7:25:7:32 | EnvValue |
17+
| test.js:8:24:8:31 | EnvValue |
18+
| test.js:8:24:8:31 | EnvValue |
19+
edges
20+
| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:11:5:18 | EnvValue |
21+
| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:21:5:26 | EnvKey |
22+
| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey |
23+
| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey |
24+
| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey |
25+
| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey |
26+
| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue |
27+
| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue |
28+
| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue |
29+
| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue |
30+
| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue |
31+
| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue |
32+
| test.js:5:11:5:18 | EnvValue | test.js:5:9:5:39 | EnvValue |
33+
| test.js:5:21:5:26 | EnvKey | test.js:5:9:5:39 | EnvKey |
34+
| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } |
35+
| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } |
36+
#select
37+
| test.js:6:25:6:32 | EnvValue | test.js:5:32:5:39 | req.body | test.js:6:25:6:32 | EnvValue | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source |
38+
| test.js:7:25:7:32 | EnvValue | test.js:5:32:5:39 | req.body | test.js:7:25:7:32 | EnvValue | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source |
39+
| test.js:8:24:8:31 | EnvValue | test.js:5:32:5:39 | req.body | test.js:8:24:8:31 | EnvValue | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source |
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE-099/EnvValueAndKeyInjection.ql

0 commit comments

Comments
 (0)