Merge pull request #15060 from am0o0/amammad-js-envinjection

JS: Env Injection query
2026-04-24 08:15:14 +02:00 · 2024-06-20 21:27:21 +02:00
parent 555d7e5958 4e1f7a930d
commit db768960f4
12 changed files with 300 additions and 0 deletions
--- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp
+++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.qhelp
@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+	<overview>
+
+		<p>
+			Controlling the value of arbitrary environment variables from user-controllable data is not safe.
+		</p>
+
+	</overview>
+
+	<recommendation>
+
+		<p>
+			Restrict this operation only to privileged users or only for some not important environment variables.
+		</p>
+
+	</recommendation>
+
+	<example>
+
+		<p>
+			The following example allows unauthorized users to assign a value to any environment variable.
+		</p>
+
+		<sample src="examples/Bad_Value_And_Key_Assignment.js" />
+
+	</example>
+
+	<references>
+		<li> <a href="https://huntr.com/bounties/00ec6847-125b-43e9-9658-d3cace1751d6/">Admin account TakeOver in mintplex-labs/anything-llm</a></li>
+	</references>
+
+</qhelp>
--- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql
+++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql
@@ -0,0 +1,69 @@
+/**
+ * @name User controlled arbitrary environment variable injection
+ * @description creating arbitrary environment variables from user controlled data is not secure
+ * @kind path-problem
+ * @id js/env-key-and-value-injection
+ * @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 = keyOfEnv() or
+    sink = valueOfEnv()
+  }
+
+  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)
+      )
+    )
+  }
+}
+
+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 cfgForValue, Configuration cfgForKey, DataFlow::PathNode source,
+  DataFlow::PathNode envKey, DataFlow::PathNode envValue
+where
+  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"
--- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp
+++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.qhelp
@@ -0,0 +1,36 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+	<overview>
+
+		<p>
+			Assigning Value to environment variables from user-controllable data is not safe.
+		</p>
+
+	</overview>
+
+	<recommendation>
+
+		<p>
+			Restrict this operation only to privileged users or only for some not important environment variables.
+		</p>
+
+	</recommendation>
+
+	<example>
+
+		<p>
+			The following example allows unauthorized users to assign a value to a critical environment variable.
+		</p>
+
+		<sample src="examples/Bad_Value_Assignment.js" />
+
+	</example>
+
+	<references>
+		<li><a href="https://huntr.com/bounties/00ec6847-125b-43e9-9658-d3cace1751d6/">Admin account TakeOver in mintplex-labs/anything-llm</a></li>
+	</references>
+
+</qhelp>
--- a/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.ql
+++ b/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.ql
@@ -0,0 +1,30 @@
+/**
+ * @name User controlled environment variable value injection
+ * @description assigning important environment variables from user controlled data is not secure
+ * @kind path-problem
+ * @id js/env-value-injection
+ * @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"
--- 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
@@ -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!');
+});
--- a/javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_Assignment.js
+++ b/javascript/ql/src/experimental/Security/CWE-099/examples/Bad_Value_Assignment.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!');
+});
--- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected
+++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.expected
@@ -0,0 +1,55 @@
+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: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 |
+| 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: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: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 |
--- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.qlref
+++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/EnvValueAndKeyInjection.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-099/EnvValueAndKeyInjection.ql
--- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js
+++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueAndKeyInjection/test.js
@@ -0,0 +1,19 @@
+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
+
+  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!');
+});
--- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.expected
+++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.expected
@@ -0,0 +1,27 @@
+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 |
+| 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 |
--- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.qlref
+++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/EnvValueInjection.qlref
@@ -0,0 +1 @@
+experimental/Security/CWE-099/EnvValueInjection.ql
--- a/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/test.js
+++ b/javascript/ql/test/experimental/Security/CWE-099/EnvValueInjection/test.js
@@ -0,0 +1,10 @@
+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
+  process.env.AKey = EnvValue; // NOT OK
+
+  res.end('env has been injected!');
+});
				`@@ -0,0 +1 @@`
				`experimental/Security/CWE-099/EnvValueAndKeyInjection.ql`
				`@@ -0,0 +1 @@`
				`experimental/Security/CWE-099/EnvValueInjection.ql`