Skip to content

Commit 18d0b28

Browse files
am0o0am0o0
committed
v1
1 parent 4ef1fe4 commit 18d0b28

7 files changed

Lines changed: 171 additions & 0 deletions

File tree

‎javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp‎

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+
Assigning Value to 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 a critical environment variable.
26+
</p>
27+
28+
<sample src="examples/Bad.js" />
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/EnvInjection.ql‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* @name User controlled environment injection
3+
* @description full control on creating environment variables from user controlled data is not secure
4+
* @kind path-problem
5+
* @id js/envinjection
6+
* @problem.severity error
7+
* @security-severity 7.5
8+
* @precision medium
9+
* @tags security
10+
* external/cwe/cwe-089
11+
*/
12+
13+
import javascript
14+
import DataFlow::PathGraph
15+
16+
/** A taint tracking configuration for unsafe environment injection. */
17+
class Configuration extends TaintTracking::Configuration {
18+
Configuration() { this = "envInjection" }
19+
20+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
21+
22+
override predicate isSink(DataFlow::Node sink) {
23+
sink = API::moduleImport("process").getMember("env").getAMember().asSink()
24+
}
25+
}
26+
27+
28+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
29+
where cfg.hasFlowPath(source, sink)
30+
select sink.getNode(), source, sink, "this environment variable assignment is $@.",
31+
source.getNode(), "user controllable"

‎javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* @name User controlled environment injection
3+
* @description full control on creating environment variables from user controlled data is not secure
4+
* @kind path-problem
5+
* @id js/envinjection
6+
* @problem.severity error
7+
* @security-severity 7.5
8+
* @precision medium
9+
* @tags security
10+
* external/cwe/cwe-089
11+
*/
12+
13+
import javascript
14+
import DataFlow::PathGraph
15+
16+
/** A taint tracking configuration for unsafe environment injection. */
17+
class Configuration extends TaintTracking::Configuration {
18+
Configuration() { this = "envInjection" }
19+
20+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
21+
22+
override predicate isSink(DataFlow::Node sink) {
23+
NodeJSLib::process()
24+
.getAPropertyRead("env")
25+
.asExpr()
26+
.getParent()
27+
.(IndexExpr)
28+
.getAChildExpr()
29+
.(VarRef) = sink.asExpr()
30+
or
31+
sink = API::moduleImport("process").getMember("env").getAMember().asSink()
32+
}
33+
34+
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
35+
exists(DataFlow::InvokeNode ikn |
36+
ikn = DataFlow::globalVarRef("Object").getAMemberInvocation("keys")
37+
|
38+
pred = ikn.getArgument(0) and
39+
(
40+
succ = ikn.getAChainedMethodCall(["filter", "map"]) or
41+
succ = ikn or
42+
succ = ikn.getAChainedMethodCall("forEach").getABoundCallbackParameter(0, 0)
43+
)
44+
)
45+
}
46+
}
47+
48+
from
49+
Configuration cfg, Configuration cfg2, DataFlow::PathNode source, DataFlow::PathNode sink,
50+
DataFlow::PathNode sink2
51+
where
52+
cfg.hasFlowPath(source, sink) and
53+
sink.getNode() = API::moduleImport("process").getMember("env").getAMember().asSink() and
54+
cfg2.hasFlowPath(source, sink2) and
55+
sink.getNode().asExpr() =
56+
NodeJSLib::process()
57+
.getAPropertyRead("env")
58+
.asExpr()
59+
.getParent()
60+
.(IndexExpr)
61+
.getAChildExpr()
62+
.(VarRef)
63+
select sink.getNode(), source, sink, "this environment variable assignment is $@.",
64+
source.getNode(), "user controllable"

‎javascript/ql/src/experimental/Security/CWE-099/examples/Bad.js‎

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 } = req.body;
5+
process.env["A_Critical_Env"] = EnvValue; // NOT OK
6+
7+
res.end('env has been injected!');
8+
});

‎javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
nodes
2+
| test.js:4:9:4:20 | { EnvValue } |
3+
| test.js:4:9:4:31 | EnvValue |
4+
| test.js:4:11:4:18 | EnvValue |
5+
| test.js:4:24:4:31 | req.body |
6+
| test.js:4:24:4:31 | req.body |
7+
| test.js:5:35:5:42 | EnvValue |
8+
| test.js:5:35:5:42 | EnvValue |
9+
| test.js:6:23:6:30 | EnvValue |
10+
| test.js:6:23:6:30 | EnvValue |
11+
edges
12+
| test.js:4:9:4:20 | { EnvValue } | test.js:4:11:4:18 | EnvValue |
13+
| test.js:4:9:4:31 | EnvValue | test.js:5:35:5:42 | EnvValue |
14+
| test.js:4:9:4:31 | EnvValue | test.js:5:35:5:42 | EnvValue |
15+
| test.js:4:9:4:31 | EnvValue | test.js:6:23:6:30 | EnvValue |
16+
| test.js:4:9:4:31 | EnvValue | test.js:6:23:6:30 | EnvValue |
17+
| test.js:4:11:4:18 | EnvValue | test.js:4:9:4:31 | EnvValue |
18+
| test.js:4:24:4:31 | req.body | test.js:4:9:4:20 | { EnvValue } |
19+
| test.js:4:24:4:31 | req.body | test.js:4:9:4:20 | { EnvValue } |
20+
#select
21+
| test.js:5:35:5:42 | EnvValue | test.js:4:24:4:31 | req.body | test.js:5:35:5:42 | EnvValue | this environment variable assignment is $@. | test.js:4:24:4:31 | req.body | user controllable |
22+
| test.js:6:23:6:30 | EnvValue | test.js:4:24:4:31 | req.body | test.js:6:23:6:30 | EnvValue | this environment variable assignment is $@. | test.js:4:24:4:31 | req.body | user controllable |

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

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

‎javascript/ql/test/experimental/Security/CWE-099/test.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
const http = require('node:http');
2+
3+
http.createServer((req, res) => {
4+
const { EnvValue } = req.body;
5+
process.env["A_Critical_Env"] = EnvValue; // NOT OK
6+
process.env[AKey] = EnvValue; // NOT OK
7+
8+
res.end('env has been injected!');
9+
});

0 commit comments

Comments
 (0)