From 18d0b2802455a025129685b7628886d0d5d38be6 Mon Sep 17 00:00:00 2001 From: amammad <77095239+amammad@users.noreply.github.com> Date: Sun, 10 Dec 2023 20:27:21 +0100 Subject: [PATCH 1/9] v1 --- .../Security/CWE-099/EnvInjection.qhelp | 36 +++++++++++ .../Security/CWE-099/EnvInjection.ql | 31 +++++++++ .../CWE-099/EnvInjectionWithKeyControl.ql | 64 +++++++++++++++++++ .../Security/CWE-099/examples/Bad.js | 8 +++ .../Security/CWE-099/EnvInjection.expected | 22 +++++++ .../Security/CWE-099/EnvInjection.qlref | 1 + .../experimental/Security/CWE-099/test.js | 9 +++ 7 files changed, 171 insertions(+) create mode 100644 javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp create mode 100644 javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql create mode 100644 javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql create mode 100644 javascript/ql/src/experimental/Security/CWE-099/examples/Bad.js create mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected create mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref create mode 100644 javascript/ql/test/experimental/Security/CWE-099/test.js diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp b/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp new file mode 100644 index 000000000000..65c324b16d78 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp @@ -0,0 +1,36 @@ + + + + + +

+ Assigning Value to environment variables from user-controllable data is not safe. +

+ + + + + +

+ Restrict this operation only to privileged users or only for some not important environment variables. +

+ + + + + +

+ The following example allows unauthorized users to assign a value to a critical environment variable. +

+ + + + + + + Admin account TakeOver in mintplex-labs/anything-llm + + + diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql b/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql new file mode 100644 index 000000000000..a5bb557a2f20 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql @@ -0,0 +1,31 @@ +/** + * @name User controlled environment injection + * @description full control on creating environment variables from user controlled data is not secure + * @kind path-problem + * @id js/envinjection + * @problem.severity error + * @security-severity 7.5 + * @precision medium + * @tags security + * external/cwe/cwe-089 + */ + +import javascript +import DataFlow::PathGraph + +/** A taint tracking configuration for unsafe environment injection. */ +class Configuration extends TaintTracking::Configuration { + Configuration() { this = "envInjection" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { + sink = API::moduleImport("process").getMember("env").getAMember().asSink() + } +} + + +from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink +where cfg.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "this environment variable assignment is $@.", + source.getNode(), "user controllable" diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql b/javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql new file mode 100644 index 000000000000..02105cf51819 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql @@ -0,0 +1,64 @@ +/** + * @name User controlled environment injection + * @description full control on creating environment variables from user controlled data is not secure + * @kind path-problem + * @id js/envinjection + * @problem.severity error + * @security-severity 7.5 + * @precision medium + * @tags security + * external/cwe/cwe-089 + */ + +import javascript +import DataFlow::PathGraph + +/** A taint tracking configuration for unsafe environment injection. */ +class Configuration extends TaintTracking::Configuration { + Configuration() { this = "envInjection" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { + NodeJSLib::process() + .getAPropertyRead("env") + .asExpr() + .getParent() + .(IndexExpr) + .getAChildExpr() + .(VarRef) = sink.asExpr() + or + sink = API::moduleImport("process").getMember("env").getAMember().asSink() + } + + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::InvokeNode ikn | + ikn = DataFlow::globalVarRef("Object").getAMemberInvocation("keys") + | + pred = ikn.getArgument(0) and + ( + succ = ikn.getAChainedMethodCall(["filter", "map"]) or + succ = ikn or + succ = ikn.getAChainedMethodCall("forEach").getABoundCallbackParameter(0, 0) + ) + ) + } +} + +from + Configuration cfg, Configuration cfg2, DataFlow::PathNode source, DataFlow::PathNode sink, + DataFlow::PathNode sink2 +where + cfg.hasFlowPath(source, sink) and + sink.getNode() = API::moduleImport("process").getMember("env").getAMember().asSink() and + cfg2.hasFlowPath(source, sink2) and + sink.getNode().asExpr() = + NodeJSLib::process() + .getAPropertyRead("env") + .asExpr() + .getParent() + .(IndexExpr) + .getAChildExpr() + .(VarRef) +select sink.getNode(), source, sink, "this environment variable assignment is $@.", + source.getNode(), "user controllable" diff --git a/javascript/ql/src/experimental/Security/CWE-099/examples/Bad.js b/javascript/ql/src/experimental/Security/CWE-099/examples/Bad.js new file mode 100644 index 000000000000..6f0cc7fc4290 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-099/examples/Bad.js @@ -0,0 +1,8 @@ +const http = require('node:http'); + +http.createServer((req, res) => { + const { EnvValue } = req.body; + process.env["A_Critical_Env"] = EnvValue; // NOT OK + + res.end('env has been injected!'); +}); \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected new file mode 100644 index 000000000000..39b8fe76100f --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected @@ -0,0 +1,22 @@ +nodes +| test.js:4:9:4:20 | { EnvValue } | +| test.js:4:9:4:31 | EnvValue | +| test.js:4:11:4:18 | EnvValue | +| test.js:4:24:4:31 | req.body | +| test.js:4:24:4:31 | req.body | +| test.js:5:35:5:42 | EnvValue | +| test.js:5:35:5:42 | EnvValue | +| test.js:6:23:6:30 | EnvValue | +| test.js:6:23:6:30 | EnvValue | +edges +| test.js:4:9:4:20 | { EnvValue } | test.js:4:11:4:18 | EnvValue | +| test.js:4:9:4:31 | EnvValue | test.js:5:35:5:42 | EnvValue | +| test.js:4:9:4:31 | EnvValue | test.js:5:35:5:42 | EnvValue | +| test.js:4:9:4:31 | EnvValue | test.js:6:23:6:30 | EnvValue | +| test.js:4:9:4:31 | EnvValue | test.js:6:23:6:30 | EnvValue | +| test.js:4:11:4:18 | EnvValue | test.js:4:9:4:31 | EnvValue | +| test.js:4:24:4:31 | req.body | test.js:4:9:4:20 | { EnvValue } | +| test.js:4:24:4:31 | req.body | test.js:4:9:4:20 | { EnvValue } | +#select +| 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 | +| 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 | diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref b/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref new file mode 100644 index 000000000000..db98debeccdd --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-099/EnvInjection.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-099/test.js b/javascript/ql/test/experimental/Security/CWE-099/test.js new file mode 100644 index 000000000000..6e592e31ff25 --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/test.js @@ -0,0 +1,9 @@ +const http = require('node:http'); + +http.createServer((req, res) => { + const { EnvValue } = req.body; + process.env["A_Critical_Env"] = EnvValue; // NOT OK + process.env[AKey] = EnvValue; // NOT OK + + res.end('env has been injected!'); +}); \ No newline at end of file From 102f09aa2348a6a2aef97ab3a7c94f5213153069 Mon Sep 17 00:00:00 2001 From: amammad <77095239+amammad@users.noreply.github.com> Date: Sun, 10 Dec 2023 20:33:00 +0100 Subject: [PATCH 2/9] extend tests --- .../test/experimental/Security/CWE-099/EnvInjection.expected | 5 +++++ javascript/ql/test/experimental/Security/CWE-099/test.js | 1 + 2 files changed, 6 insertions(+) diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected index 39b8fe76100f..7461f72ee7eb 100644 --- a/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected @@ -8,15 +8,20 @@ nodes | test.js:5:35:5:42 | EnvValue | | test.js:6:23:6:30 | EnvValue | | test.js:6:23:6:30 | EnvValue | +| test.js:7:22:7:29 | EnvValue | +| test.js:7:22:7:29 | EnvValue | edges | test.js:4:9:4:20 | { EnvValue } | test.js:4:11:4:18 | EnvValue | | test.js:4:9:4:31 | EnvValue | test.js:5:35:5:42 | EnvValue | | test.js:4:9:4:31 | EnvValue | test.js:5:35:5:42 | EnvValue | | test.js:4:9:4:31 | EnvValue | test.js:6:23:6:30 | EnvValue | | test.js:4:9:4:31 | EnvValue | test.js:6:23:6:30 | EnvValue | +| test.js:4:9:4:31 | EnvValue | test.js:7:22:7:29 | EnvValue | +| test.js:4:9:4:31 | EnvValue | test.js:7:22:7:29 | EnvValue | | test.js:4:11:4:18 | EnvValue | test.js:4:9:4:31 | EnvValue | | test.js:4:24:4:31 | req.body | test.js:4:9:4:20 | { EnvValue } | | test.js:4:24:4:31 | req.body | test.js:4:9:4:20 | { EnvValue } | #select | 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 | | 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 | +| test.js:7:22:7:29 | EnvValue | test.js:4:24:4:31 | req.body | test.js:7:22:7:29 | EnvValue | this environment variable assignment is $@. | test.js:4:24:4:31 | req.body | user controllable | diff --git a/javascript/ql/test/experimental/Security/CWE-099/test.js b/javascript/ql/test/experimental/Security/CWE-099/test.js index 6e592e31ff25..9d034cfed904 100644 --- a/javascript/ql/test/experimental/Security/CWE-099/test.js +++ b/javascript/ql/test/experimental/Security/CWE-099/test.js @@ -4,6 +4,7 @@ http.createServer((req, res) => { const { EnvValue } = req.body; process.env["A_Critical_Env"] = EnvValue; // NOT OK process.env[AKey] = EnvValue; // NOT OK + process.env.AKey = EnvValue; // NOT OK res.end('env has been injected!'); }); \ No newline at end of file From 1fc481ce81e46f234bef8dbfca86b60a29452fc9 Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Sat, 25 May 2024 20:43:36 +0200 Subject: [PATCH 3/9] v2: it is basically the first stable version :)) --- .../CWE-099/EnvValueAndKeyInjection.qhelp | 36 +++++++++++++++++ ...yControl.ql => EnvValueAndKeyInjection.ql} | 18 ++++----- ...njection.qhelp => EnvValueInjection.qhelp} | 2 +- .../{EnvInjection.ql => EnvValueInjection.ql} | 7 ++-- .../examples/Bad_Value_And_Key_Assignment.js | 8 ++++ .../{Bad.js => Bad_Value_Assignment.js} | 0 .../Security/CWE-099/EnvInjection.qlref | 1 - .../EnvInjection.expected | 39 +++++++++++++++++++ .../EnvValueAndKeyInjection.expected | 39 +++++++++++++++++++ .../EnvValueAndKeyInjection.qlref | 1 + .../CWE-099/EnvValueAndKeyInjection/test.js | 11 ++++++ .../EnvValueInjection.expected} | 0 .../EnvValueInjection/EnvValueInjection.qlref | 1 + .../CWE-099/{ => EnvValueInjection}/test.js | 2 +- 14 files changed, 149 insertions(+), 16 deletions(-) create mode 100644 javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp rename javascript/ql/src/experimental/Security/CWE-099/{EnvInjectionWithKeyControl.ql => EnvValueAndKeyInjection.ql} (74%) rename javascript/ql/src/experimental/Security/CWE-099/{EnvInjection.qhelp => EnvValueInjection.qhelp} (92%) rename javascript/ql/src/experimental/Security/CWE-099/{EnvInjection.ql => EnvValueInjection.ql} (81%) create mode 100644 javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_And_Key_Assignment.js rename javascript/ql/src/experimental/Security/CWE-099/examples/{Bad.js => Bad_Value_Assignment.js} (100%) delete mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref create mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected create mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected create mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.qlref create mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js rename javascript/ql/test/experimental/Security/CWE-099/{EnvInjection.expected => EnvValueInjection/EnvValueInjection.expected} (100%) create mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.qlref rename javascript/ql/test/experimental/Security/CWE-099/{ => EnvValueInjection}/test.js (98%) diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp new file mode 100644 index 000000000000..fbf9bf81e733 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp @@ -0,0 +1,36 @@ + + + + + +

+ Controlling the value of arbitrary environment variables from user-controllable data is not safe. +

+ + + + + +

+ Restrict this operation only to privileged users or only for some not important environment variables. +

+ + + + + +

+ The following example allows unauthorized users to assign a value to any environment variable. +

+ + + + + + + Admin account TakeOver in mintplex-labs/anything-llm + + + diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql similarity index 74% rename from javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql rename to javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql index 02105cf51819..152552279cd6 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvInjectionWithKeyControl.ql +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql @@ -1,8 +1,8 @@ /** - * @name User controlled environment injection - * @description full control on creating environment variables from user controlled data is not secure + * @name User controlled arbitrary environment variable injection + * @description creating arbitrary environment variables from user controlled data is not secure * @kind path-problem - * @id js/envinjection + * @id js/env-key-and-value-injection * @problem.severity error * @security-severity 7.5 * @precision medium @@ -46,13 +46,13 @@ class Configuration extends TaintTracking::Configuration { } from - Configuration cfg, Configuration cfg2, DataFlow::PathNode source, DataFlow::PathNode sink, + Configuration cfg, Configuration cfg2, DataFlow::PathNode source, DataFlow::PathNode sink1, DataFlow::PathNode sink2 where - cfg.hasFlowPath(source, sink) and - sink.getNode() = API::moduleImport("process").getMember("env").getAMember().asSink() and + cfg.hasFlowPath(source, sink1) and + sink1.getNode() = API::moduleImport("process").getMember("env").getAMember().asSink() and cfg2.hasFlowPath(source, sink2) and - sink.getNode().asExpr() = + sink2.getNode().asExpr() = NodeJSLib::process() .getAPropertyRead("env") .asExpr() @@ -60,5 +60,5 @@ where .(IndexExpr) .getAChildExpr() .(VarRef) -select sink.getNode(), source, sink, "this environment variable assignment is $@.", - source.getNode(), "user controllable" +select sink1.getNode(), source, sink1, "arbitrary environment variable assignment from this $@.", + source.getNode(), "user controllable source" diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp similarity index 92% rename from javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp rename to javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp index 65c324b16d78..cbc6a04297d4 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.qhelp +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp @@ -25,7 +25,7 @@ The following example allows unauthorized users to assign a value to a critical environment variable.

- + diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.ql similarity index 81% rename from javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql rename to javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.ql index a5bb557a2f20..3eb9f2305644 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvInjection.ql +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.ql @@ -1,8 +1,8 @@ /** - * @name User controlled environment injection - * @description full control on creating environment variables from user controlled data is not secure + * @name User controlled environment variable value injection + * @description assigning important environment variables from user controlled data is not secure * @kind path-problem - * @id js/envinjection + * @id js/env-value-injection * @problem.severity error * @security-severity 7.5 * @precision medium @@ -24,7 +24,6 @@ class Configuration extends TaintTracking::Configuration { } } - from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink where cfg.hasFlowPath(source, sink) select sink.getNode(), source, sink, "this environment variable assignment is $@.", diff --git a/javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_And_Key_Assignment.js b/javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_And_Key_Assignment.js new file mode 100644 index 000000000000..a6553f9c0ff3 --- /dev/null +++ b/javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_And_Key_Assignment.js @@ -0,0 +1,8 @@ +const http = require('node:http'); + +http.createServer((req, res) => { + const { EnvValue, EnvKey } = req.body; + process.env[EnvKey] = EnvValue; // NOT OK + + res.end('env has been injected!'); +}); \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-099/examples/Bad.js b/javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_Assignment.js similarity index 100% rename from javascript/ql/src/experimental/Security/CWE-099/examples/Bad.js rename to javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_Assignment.js diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref b/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref deleted file mode 100644 index db98debeccdd..000000000000 --- a/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE-099/EnvInjection.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected new file mode 100644 index 000000000000..9659366f6f00 --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected @@ -0,0 +1,39 @@ +nodes +| test.js:5:9:5:28 | { EnvValue, EnvKey } | +| test.js:5:9:5:39 | EnvKey | +| test.js:5:9:5:39 | EnvValue | +| test.js:5:11:5:18 | EnvValue | +| test.js:5:21:5:26 | EnvKey | +| test.js:5:32:5:39 | req.body | +| test.js:5:32:5:39 | req.body | +| test.js:6:15:6:20 | EnvKey | +| test.js:6:15:6:20 | EnvKey | +| test.js:6:25:6:32 | EnvValue | +| test.js:6:25:6:32 | EnvValue | +| test.js:7:15:7:20 | EnvKey | +| test.js:7:15:7:20 | EnvKey | +| test.js:7:25:7:32 | EnvValue | +| test.js:7:25:7:32 | EnvValue | +| test.js:8:24:8:31 | EnvValue | +| test.js:8:24:8:31 | EnvValue | +edges +| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:11:5:18 | EnvValue | +| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:21:5:26 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey | +| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | +| test.js:5:11:5:18 | EnvValue | test.js:5:9:5:39 | EnvValue | +| test.js:5:21:5:26 | EnvKey | test.js:5:9:5:39 | EnvKey | +| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | +| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | +#select +| 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 | +| 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 | +| 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 | diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected new file mode 100644 index 000000000000..9659366f6f00 --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected @@ -0,0 +1,39 @@ +nodes +| test.js:5:9:5:28 | { EnvValue, EnvKey } | +| test.js:5:9:5:39 | EnvKey | +| test.js:5:9:5:39 | EnvValue | +| test.js:5:11:5:18 | EnvValue | +| test.js:5:21:5:26 | EnvKey | +| test.js:5:32:5:39 | req.body | +| test.js:5:32:5:39 | req.body | +| test.js:6:15:6:20 | EnvKey | +| test.js:6:15:6:20 | EnvKey | +| test.js:6:25:6:32 | EnvValue | +| test.js:6:25:6:32 | EnvValue | +| test.js:7:15:7:20 | EnvKey | +| test.js:7:15:7:20 | EnvKey | +| test.js:7:25:7:32 | EnvValue | +| test.js:7:25:7:32 | EnvValue | +| test.js:8:24:8:31 | EnvValue | +| test.js:8:24:8:31 | EnvValue | +edges +| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:11:5:18 | EnvValue | +| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:21:5:26 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey | +| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey | +| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | +| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | +| test.js:5:11:5:18 | EnvValue | test.js:5:9:5:39 | EnvValue | +| test.js:5:21:5:26 | EnvKey | test.js:5:9:5:39 | EnvKey | +| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | +| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | +#select +| 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 | +| 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 | +| 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 | diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.qlref b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.qlref new file mode 100644 index 000000000000..fde9a286e5a8 --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-099/EnvValueAndKeyInjection.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js new file mode 100644 index 000000000000..c8a9b6ffabaa --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js @@ -0,0 +1,11 @@ +const http = require('node:http'); + + +http.createServer((req, res) => { + const { EnvValue, EnvKey } = req.body; + process.env[EnvKey] = EnvValue; // NOT OK + process.env[EnvKey] = EnvValue; // NOT OK + process.env.EnvKey = EnvValue; // NOT OK + + res.end('env has been injected!'); +}); \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.expected similarity index 100% rename from javascript/ql/test/experimental/Security/CWE-099/EnvInjection.expected rename to javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.expected diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.qlref b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.qlref new file mode 100644 index 000000000000..e03328beda4f --- /dev/null +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.qlref @@ -0,0 +1 @@ +experimental/Security/CWE-099/EnvValueInjection.ql \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-099/test.js b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/test.js similarity index 98% rename from javascript/ql/test/experimental/Security/CWE-099/test.js rename to javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/test.js index 9d034cfed904..cb28f01b88b0 100644 --- a/javascript/ql/test/experimental/Security/CWE-099/test.js +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/test.js @@ -7,4 +7,4 @@ http.createServer((req, res) => { process.env.AKey = EnvValue; // NOT OK res.end('env has been injected!'); -}); \ No newline at end of file +}); From 2b929c4d2d1d406c47afb1d7de078df02f78027b Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Sat, 25 May 2024 20:45:34 +0200 Subject: [PATCH 4/9] remove old expected test file --- .../EnvInjection.expected | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected deleted file mode 100644 index 9659366f6f00..000000000000 --- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvInjection.expected +++ /dev/null @@ -1,39 +0,0 @@ -nodes -| test.js:5:9:5:28 | { EnvValue, EnvKey } | -| test.js:5:9:5:39 | EnvKey | -| test.js:5:9:5:39 | EnvValue | -| test.js:5:11:5:18 | EnvValue | -| test.js:5:21:5:26 | EnvKey | -| test.js:5:32:5:39 | req.body | -| test.js:5:32:5:39 | req.body | -| test.js:6:15:6:20 | EnvKey | -| test.js:6:15:6:20 | EnvKey | -| test.js:6:25:6:32 | EnvValue | -| test.js:6:25:6:32 | EnvValue | -| test.js:7:15:7:20 | EnvKey | -| test.js:7:15:7:20 | EnvKey | -| test.js:7:25:7:32 | EnvValue | -| test.js:7:25:7:32 | EnvValue | -| test.js:8:24:8:31 | EnvValue | -| test.js:8:24:8:31 | EnvValue | -edges -| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:11:5:18 | EnvValue | -| test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:21:5:26 | EnvKey | -| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey | -| test.js:5:9:5:39 | EnvKey | test.js:6:15:6:20 | EnvKey | -| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey | -| test.js:5:9:5:39 | EnvKey | test.js:7:15:7:20 | EnvKey | -| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue | -| test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue | -| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | -| test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | -| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | -| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | -| test.js:5:11:5:18 | EnvValue | test.js:5:9:5:39 | EnvValue | -| test.js:5:21:5:26 | EnvKey | test.js:5:9:5:39 | EnvKey | -| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | -| test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | -#select -| 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 | -| 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 | -| 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 | From 5e0a78c4c7c4ab61180be88816838faa029bc138 Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Fri, 7 Jun 2024 21:15:03 +0200 Subject: [PATCH 5/9] make predicate for env key and value nodes, use propertyRead/Write instead of API nodes to find env key and value assignments, fix a bug thanks to @erik-krogh --- .../CWE-099/EnvValueAndKeyInjection.ql | 51 ++++++++++--------- .../CWE-099/EnvValueAndKeyInjection/test.js | 9 ++++ 2 files changed, 37 insertions(+), 23 deletions(-) diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql index 152552279cd6..38c7e5912110 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql @@ -20,15 +20,8 @@ class Configuration extends TaintTracking::Configuration { override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { - NodeJSLib::process() - .getAPropertyRead("env") - .asExpr() - .getParent() - .(IndexExpr) - .getAChildExpr() - .(VarRef) = sink.asExpr() - or - sink = API::moduleImport("process").getMember("env").getAMember().asSink() + sink = keyOfEnv() or + sink = valueOfEnv() } override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { @@ -45,20 +38,32 @@ class Configuration extends TaintTracking::Configuration { } } +DataFlow::Node keyOfEnv() { + result = + NodeJSLib::process().getAPropertyRead("env").getAPropertyWrite().getPropertyNameExpr().flow() +} + +DataFlow::Node valueOfEnv() { + result = API::moduleImport("process").getMember("env").getAMember().asSink() +} + +private predicate readToProcessEnv(DataFlow::Node envKey, DataFlow::Node envValue) { + exists(DataFlow::PropWrite env | + env = NodeJSLib::process().getAPropertyRead("env").getAPropertyWrite() + | + envKey = env.getPropertyNameExpr().flow() and + envValue = env.getRhs() + ) +} + from - Configuration cfg, Configuration cfg2, DataFlow::PathNode source, DataFlow::PathNode sink1, - DataFlow::PathNode sink2 + Configuration cfgForValue, Configuration cfgForKey, DataFlow::PathNode source, + DataFlow::PathNode envKey, DataFlow::PathNode envValue where - cfg.hasFlowPath(source, sink1) and - sink1.getNode() = API::moduleImport("process").getMember("env").getAMember().asSink() and - cfg2.hasFlowPath(source, sink2) and - sink2.getNode().asExpr() = - NodeJSLib::process() - .getAPropertyRead("env") - .asExpr() - .getParent() - .(IndexExpr) - .getAChildExpr() - .(VarRef) -select sink1.getNode(), source, sink1, "arbitrary environment variable assignment from this $@.", + cfgForValue.hasFlowPath(source, envKey) and + envKey.getNode() = keyOfEnv() and + cfgForKey.hasFlowPath(source, envValue) and + envValue.getNode() = valueOfEnv() and + readToProcessEnv(envKey.getNode(), envValue.getNode()) +select envKey.getNode(), source, envKey, "arbitrary environment variable assignment from this $@.", source.getNode(), "user controllable source" diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js index c8a9b6ffabaa..7938f90e180f 100644 --- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js @@ -7,5 +7,14 @@ http.createServer((req, res) => { process.env[EnvKey] = EnvValue; // NOT OK process.env.EnvKey = EnvValue; // NOT OK + res.end('env has been injected!'); +}); + +http.createServer((req, res) => { + const { EnvValue, EnvKey } = req.body; + + process.env[EnvKey] = "constant" // OK + process.env.constant = EnvValue // OK + res.end('env has been injected!'); }); \ No newline at end of file From 2c9340331dab13ba2e1c06dff35623a3b6e56e07 Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Fri, 7 Jun 2024 21:16:27 +0200 Subject: [PATCH 6/9] update test cases expected results --- .../EnvValueAndKeyInjection.expected | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected index 9659366f6f00..c78c367fe1bb 100644 --- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected @@ -16,6 +16,17 @@ nodes | test.js:7:25:7:32 | EnvValue | | test.js:8:24:8:31 | EnvValue | | test.js:8:24:8:31 | EnvValue | +| test.js:14:9:14:28 | { EnvValue, EnvKey } | +| test.js:14:9:14:39 | EnvKey | +| test.js:14:9:14:39 | EnvValue | +| test.js:14:11:14:18 | EnvValue | +| test.js:14:21:14:26 | EnvKey | +| test.js:14:32:14:39 | req.body | +| test.js:14:32:14:39 | req.body | +| test.js:16:15:16:20 | EnvKey | +| test.js:16:15:16:20 | EnvKey | +| test.js:17:26:17:33 | EnvValue | +| test.js:17:26:17:33 | EnvValue | edges | test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:11:5:18 | EnvValue | | test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:21:5:26 | EnvKey | @@ -33,7 +44,16 @@ edges | test.js:5:21:5:26 | EnvKey | test.js:5:9:5:39 | EnvKey | | test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | | test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | +| test.js:14:9:14:28 | { EnvValue, EnvKey } | test.js:14:11:14:18 | EnvValue | +| test.js:14:9:14:28 | { EnvValue, EnvKey } | test.js:14:21:14:26 | EnvKey | +| test.js:14:9:14:39 | EnvKey | test.js:16:15:16:20 | EnvKey | +| test.js:14:9:14:39 | EnvKey | test.js:16:15:16:20 | EnvKey | +| test.js:14:9:14:39 | EnvValue | test.js:17:26:17:33 | EnvValue | +| test.js:14:9:14:39 | EnvValue | test.js:17:26:17:33 | EnvValue | +| test.js:14:11:14:18 | EnvValue | test.js:14:9:14:39 | EnvValue | +| test.js:14:21:14:26 | EnvKey | test.js:14:9:14:39 | EnvKey | +| test.js:14:32:14:39 | req.body | test.js:14:9:14:28 | { EnvValue, EnvKey } | +| test.js:14:32:14:39 | req.body | test.js:14:9:14:28 | { EnvValue, EnvKey } | #select -| 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 | -| 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 | -| 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 | +| test.js:6:15:6:20 | EnvKey | test.js:5:32:5:39 | req.body | test.js:6:15:6:20 | EnvKey | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source | +| test.js:7:15:7:20 | EnvKey | test.js:5:32:5:39 | req.body | test.js:7:15:7:20 | EnvKey | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source | From 84b9d4d1ace3be11241be7ffe6c1be333531e481 Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Thu, 13 Jun 2024 14:32:41 +0200 Subject: [PATCH 7/9] fix qlhelp errors --- .../experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp | 2 +- .../src/experimental/Security/CWE-099/EnvValueInjection.qhelp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp index fbf9bf81e733..4fba14b7bcb1 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp @@ -30,7 +30,7 @@ - Admin account TakeOver in mintplex-labs/anything-llm +
  • Admin account TakeOver in mintplex-labs/anything-llm
    •  diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp index cbc6a04297d4..34720b0ac438 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp @@ -30,7 +30,7 @@ - Admin account TakeOver in mintplex-labs/anything-llm +
  • Admin account TakeOver in mintplex-labs/anything-llm
    •  From f0a467e80b0638065f2ff81890f41b262242dbed Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Thu, 13 Jun 2024 14:52:22 +0200 Subject: [PATCH 8/9] update tests --- .../EnvValueAndKeyInjection.expected | 46 +++++++++---------- .../CWE-099/EnvValueAndKeyInjection/test.js | 1 - 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected index c78c367fe1bb..f36626a73848 100644 --- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected @@ -14,19 +14,17 @@ nodes | test.js:7:15:7:20 | EnvKey | | test.js:7:25:7:32 | EnvValue | | test.js:7:25:7:32 | EnvValue | -| test.js:8:24:8:31 | EnvValue | -| test.js:8:24:8:31 | EnvValue | -| test.js:14:9:14:28 | { EnvValue, EnvKey } | -| test.js:14:9:14:39 | EnvKey | -| test.js:14:9:14:39 | EnvValue | -| test.js:14:11:14:18 | EnvValue | -| test.js:14:21:14:26 | EnvKey | -| test.js:14:32:14:39 | req.body | -| test.js:14:32:14:39 | req.body | -| test.js:16:15:16:20 | EnvKey | -| test.js:16:15:16:20 | EnvKey | -| test.js:17:26:17:33 | EnvValue | -| test.js:17:26:17:33 | EnvValue | +| test.js:13:9:13:28 | { EnvValue, EnvKey } | +| test.js:13:9:13:39 | EnvKey | +| test.js:13:9:13:39 | EnvValue | +| test.js:13:11:13:18 | EnvValue | +| test.js:13:21:13:26 | EnvKey | +| test.js:13:32:13:39 | req.body | +| test.js:13:32:13:39 | req.body | +| test.js:15:15:15:20 | EnvKey | +| test.js:15:15:15:20 | EnvKey | +| test.js:16:26:16:33 | EnvValue | +| test.js:16:26:16:33 | EnvValue | edges | test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:11:5:18 | EnvValue | | test.js:5:9:5:28 | { EnvValue, EnvKey } | test.js:5:21:5:26 | EnvKey | @@ -38,22 +36,20 @@ edges | test.js:5:9:5:39 | EnvValue | test.js:6:25:6:32 | EnvValue | | test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | | test.js:5:9:5:39 | EnvValue | test.js:7:25:7:32 | EnvValue | -| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | -| test.js:5:9:5:39 | EnvValue | test.js:8:24:8:31 | EnvValue | | test.js:5:11:5:18 | EnvValue | test.js:5:9:5:39 | EnvValue | | test.js:5:21:5:26 | EnvKey | test.js:5:9:5:39 | EnvKey | | test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | | test.js:5:32:5:39 | req.body | test.js:5:9:5:28 | { EnvValue, EnvKey } | -| test.js:14:9:14:28 | { EnvValue, EnvKey } | test.js:14:11:14:18 | EnvValue | -| test.js:14:9:14:28 | { EnvValue, EnvKey } | test.js:14:21:14:26 | EnvKey | -| test.js:14:9:14:39 | EnvKey | test.js:16:15:16:20 | EnvKey | -| test.js:14:9:14:39 | EnvKey | test.js:16:15:16:20 | EnvKey | -| test.js:14:9:14:39 | EnvValue | test.js:17:26:17:33 | EnvValue | -| test.js:14:9:14:39 | EnvValue | test.js:17:26:17:33 | EnvValue | -| test.js:14:11:14:18 | EnvValue | test.js:14:9:14:39 | EnvValue | -| test.js:14:21:14:26 | EnvKey | test.js:14:9:14:39 | EnvKey | -| test.js:14:32:14:39 | req.body | test.js:14:9:14:28 | { EnvValue, EnvKey } | -| test.js:14:32:14:39 | req.body | test.js:14:9:14:28 | { EnvValue, EnvKey } | +| test.js:13:9:13:28 | { EnvValue, EnvKey } | test.js:13:11:13:18 | EnvValue | +| test.js:13:9:13:28 | { EnvValue, EnvKey } | test.js:13:21:13:26 | EnvKey | +| test.js:13:9:13:39 | EnvKey | test.js:15:15:15:20 | EnvKey | +| test.js:13:9:13:39 | EnvKey | test.js:15:15:15:20 | EnvKey | +| test.js:13:9:13:39 | EnvValue | test.js:16:26:16:33 | EnvValue | +| test.js:13:9:13:39 | EnvValue | test.js:16:26:16:33 | EnvValue | +| test.js:13:11:13:18 | EnvValue | test.js:13:9:13:39 | EnvValue | +| test.js:13:21:13:26 | EnvKey | test.js:13:9:13:39 | EnvKey | +| test.js:13:32:13:39 | req.body | test.js:13:9:13:28 | { EnvValue, EnvKey } | +| test.js:13:32:13:39 | req.body | test.js:13:9:13:28 | { EnvValue, EnvKey } | #select | test.js:6:15:6:20 | EnvKey | test.js:5:32:5:39 | req.body | test.js:6:15:6:20 | EnvKey | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source | | test.js:7:15:7:20 | EnvKey | test.js:5:32:5:39 | req.body | test.js:7:15:7:20 | EnvKey | arbitrary environment variable assignment from this $@. | test.js:5:32:5:39 | req.body | user controllable source | diff --git a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js index 7938f90e180f..a12377c9cec9 100644 --- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js +++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js @@ -5,7 +5,6 @@ http.createServer((req, res) => { const { EnvValue, EnvKey } = req.body; process.env[EnvKey] = EnvValue; // NOT OK process.env[EnvKey] = EnvValue; // NOT OK - process.env.EnvKey = EnvValue; // NOT OK res.end('env has been injected!'); }); From 4e1f7a930d5cf770bc4ea12bbfd8365263971f0b Mon Sep 17 00:00:00 2001 From: am0o0 <77095239+am0o0@users.noreply.github.com> Date: Fri, 14 Jun 2024 13:47:01 +0200 Subject: [PATCH 9/9] fix invalid js file sample in qlhelp --- .../experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp index 4fba14b7bcb1..1f8ab1cc7440 100644 --- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp +++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp @@ -25,7 +25,7 @@ The following example allows unauthorized users to assign a value to any environment variable.

    - +