diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll index 1c056935d407..5ee39219d260 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll @@ -5,6 +5,7 @@ */ import javascript +private import codeql.threatmodels.ThreatModels module RegExpInjection { /** @@ -32,19 +33,32 @@ module RegExpInjection { /** * An active threat-model source, considered as a flow source. + * Excludes environment variables by default - they require the "environment" threat model. */ private class ActiveThreatModelSourceAsSource extends Source instanceof ActiveThreatModelSource { - ActiveThreatModelSourceAsSource() { not this.isClientSideSource() } + ActiveThreatModelSourceAsSource() { + not this.isClientSideSource() and + not this.(ThreatModelSource).getThreatModel() = "environment" + } } - private import IndirectCommandInjectionCustomizations + /** + * Environment variables as a source when the "environment" threat model is active. + */ + private class EnvironmentVariableAsSource extends Source instanceof ThreatModelSource { + EnvironmentVariableAsSource() { + this.getThreatModel() = "environment" and + currentThreatModel("environment") + } + + override string describe() { result = "environment variable" } + } /** - * A read of `process.env`, `process.argv`, and similar, considered as a flow source for regular - * expression injection. + * Command line arguments as a source for regular expression injection. */ - class ArgvAsSource extends Source instanceof IndirectCommandInjection::Source { - override string describe() { result = IndirectCommandInjection::Source.super.describe() } + private class CommandLineArgumentAsSource extends Source instanceof CommandLineArguments { + override string describe() { result = "command-line argument" } } /** diff --git a/javascript/ql/src/change-notes/2025-07-31-regexp-injection-threat-model.md b/javascript/ql/src/change-notes/2025-07-31-regexp-injection-threat-model.md new file mode 100644 index 000000000000..f87e10077654 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-07-31-regexp-injection-threat-model.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `js/regex-injection` query no longer considers environment variables as sources by default. Environment variables can be re-enabled as sources by setting the threat model to include the "environment" category. diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.expected similarity index 96% rename from javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.expected index 06926c487efd..07225ec763e3 100644 --- a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.expected @@ -14,7 +14,6 @@ | RegExpInjection.js:49:14:49:52 | key.spl ... in("-") | RegExpInjection.js:5:13:5:28 | req.param("key") | RegExpInjection.js:49:14:49:52 | key.spl ... in("-") | This regular expression is constructed from a $@. | RegExpInjection.js:5:13:5:28 | req.param("key") | user-provided value | | RegExpInjection.js:59:14:59:18 | input | RegExpInjection.js:55:39:55:56 | req.param("input") | RegExpInjection.js:59:14:59:18 | input | This regular expression is constructed from a $@. | RegExpInjection.js:55:39:55:56 | req.param("input") | user-provided value | | RegExpInjection.js:82:14:82:55 | "^.*\\.( ... + ")$" | RegExpInjection.js:77:15:77:32 | req.param("input") | RegExpInjection.js:82:14:82:55 | "^.*\\.( ... + ")$" | This regular expression is constructed from a $@. | RegExpInjection.js:77:15:77:32 | req.param("input") | user-provided value | -| RegExpInjection.js:86:16:86:50 | `^${pro ... r.app$` | RegExpInjection.js:86:20:86:30 | process.env | RegExpInjection.js:86:16:86:50 | `^${pro ... r.app$` | This regular expression is constructed from a $@. | RegExpInjection.js:86:20:86:30 | process.env | environment variable | | RegExpInjection.js:88:16:88:49 | `^${pro ... r.app$` | RegExpInjection.js:88:20:88:31 | process.argv | RegExpInjection.js:88:16:88:49 | `^${pro ... r.app$` | This regular expression is constructed from a $@. | RegExpInjection.js:88:20:88:31 | process.argv | command-line argument | | RegExpInjection.js:95:14:95:22 | sanitized | RegExpInjection.js:92:15:92:32 | req.param("input") | RegExpInjection.js:95:14:95:22 | sanitized | This regular expression is constructed from a $@. | RegExpInjection.js:92:15:92:32 | req.param("input") | user-provided value | | tst.js:6:16:6:35 | "^"+ data.name + "$" | tst.js:5:16:5:29 | req.query.data | tst.js:6:16:6:35 | "^"+ data.name + "$" | This regular expression is constructed from a $@. | tst.js:5:16:5:29 | req.query.data | user-provided value | @@ -57,7 +56,6 @@ edges | RegExpInjection.js:77:15:77:32 | req.param("input") | RegExpInjection.js:77:7:77:32 | input | provenance | | | RegExpInjection.js:82:25:82:29 | input | RegExpInjection.js:82:25:82:48 | input.r ... g, "\|") | provenance | | | RegExpInjection.js:82:25:82:48 | input.r ... g, "\|") | RegExpInjection.js:82:14:82:55 | "^.*\\.( ... + ")$" | provenance | | -| RegExpInjection.js:86:20:86:30 | process.env | RegExpInjection.js:86:16:86:50 | `^${pro ... r.app$` | provenance | | | RegExpInjection.js:88:20:88:31 | process.argv | RegExpInjection.js:88:16:88:49 | `^${pro ... r.app$` | provenance | | | RegExpInjection.js:92:7:92:32 | input | RegExpInjection.js:94:19:94:23 | input | provenance | | | RegExpInjection.js:92:15:92:32 | req.param("input") | RegExpInjection.js:92:7:92:32 | input | provenance | | @@ -109,8 +107,6 @@ nodes | RegExpInjection.js:82:14:82:55 | "^.*\\.( ... + ")$" | semmle.label | "^.*\\.( ... + ")$" | | RegExpInjection.js:82:25:82:29 | input | semmle.label | input | | RegExpInjection.js:82:25:82:48 | input.r ... g, "\|") | semmle.label | input.r ... g, "\|") | -| RegExpInjection.js:86:16:86:50 | `^${pro ... r.app$` | semmle.label | `^${pro ... r.app$` | -| RegExpInjection.js:86:20:86:30 | process.env | semmle.label | process.env | | RegExpInjection.js:88:16:88:49 | `^${pro ... r.app$` | semmle.label | `^${pro ... r.app$` | | RegExpInjection.js:88:20:88:31 | process.argv | semmle.label | process.argv | | RegExpInjection.js:92:7:92:32 | input | semmle.label | input | diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.js similarity index 96% rename from javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.js index 2aa73c808773..8a5fa557c19a 100644 --- a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js +++ b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.js @@ -83,7 +83,7 @@ app.get('/has-sanitizer', function(req, res) { }); app.get("argv", function(req, res) { - new RegExp(`^${process.env.HOME}/Foo/bar.app$`); // $ Alert[js/regex-injection] + new RegExp(`^${process.env.HOME}/Foo/bar.app$`); // environment variable, should be detected only with threat model enabled. new RegExp(`^${process.argv[1]}/Foo/bar.app$`); // $ Alert[js/regex-injection] }); diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.qlref b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.qlref rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjection.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjectionGood.js b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjectionGood.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/RegExpInjectionGood.js rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/RegExpInjectionGood.js diff --git a/javascript/ql/test/query-tests/Security/CWE-730/ServerCrash.expected b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/ServerCrash.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/ServerCrash.expected rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/ServerCrash.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-730/ServerCrash.qlref b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/ServerCrash.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/ServerCrash.qlref rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/ServerCrash.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-730/client-side.js b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/client-side.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/client-side.js rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/client-side.js diff --git a/javascript/ql/test/query-tests/Security/CWE-730/search.js b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/search.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/search.js rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/search.js diff --git a/javascript/ql/test/query-tests/Security/CWE-730/server-crash.js b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/server-crash.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/server-crash.js rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/server-crash.js diff --git a/javascript/ql/test/query-tests/Security/CWE-730/tst.js b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/tst.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-730/tst.js rename to javascript/ql/test/query-tests/Security/CWE-730/Threat-models-disabled/tst.js diff --git a/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.expected b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.expected new file mode 100644 index 000000000000..95c1c0df9eb8 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.expected @@ -0,0 +1,34 @@ +#select +| RegExpInjection.js:6:14:6:48 | `^${pro ... r.app$` | RegExpInjection.js:6:18:6:28 | process.env | RegExpInjection.js:6:14:6:48 | `^${pro ... r.app$` | This regular expression is constructed from a $@. | RegExpInjection.js:6:18:6:28 | process.env | environment variable | +| RegExpInjection.js:8:14:8:40 | `^${pro ... }/bin$` | RegExpInjection.js:8:18:8:28 | process.env | RegExpInjection.js:8:14:8:40 | `^${pro ... }/bin$` | This regular expression is constructed from a $@. | RegExpInjection.js:8:18:8:28 | process.env | environment variable | +| RegExpInjection.js:11:14:11:19 | envVar | RegExpInjection.js:10:16:10:26 | process.env | RegExpInjection.js:11:14:11:19 | envVar | This regular expression is constructed from a $@. | RegExpInjection.js:10:16:10:26 | process.env | environment variable | +| RegExpInjection.js:14:14:14:47 | `^${pro ... r.app$` | RegExpInjection.js:14:18:14:29 | process.argv | RegExpInjection.js:14:14:14:47 | `^${pro ... r.app$` | This regular expression is constructed from a $@. | RegExpInjection.js:14:18:14:29 | process.argv | command-line argument | +| RegExpInjection.js:17:14:17:17 | argv | RegExpInjection.js:16:14:16:25 | process.argv | RegExpInjection.js:17:14:17:17 | argv | This regular expression is constructed from a $@. | RegExpInjection.js:16:14:16:25 | process.argv | command-line argument | +| RegExpInjection.js:21:14:21:22 | userInput | RegExpInjection.js:20:19:20:36 | req.param("input") | RegExpInjection.js:21:14:21:22 | userInput | This regular expression is constructed from a $@. | RegExpInjection.js:20:19:20:36 | req.param("input") | user-provided value | +edges +| RegExpInjection.js:6:18:6:28 | process.env | RegExpInjection.js:6:14:6:48 | `^${pro ... r.app$` | provenance | | +| RegExpInjection.js:8:18:8:28 | process.env | RegExpInjection.js:8:14:8:40 | `^${pro ... }/bin$` | provenance | | +| RegExpInjection.js:10:7:10:35 | envVar | RegExpInjection.js:11:14:11:19 | envVar | provenance | | +| RegExpInjection.js:10:16:10:26 | process.env | RegExpInjection.js:10:7:10:35 | envVar | provenance | | +| RegExpInjection.js:14:18:14:29 | process.argv | RegExpInjection.js:14:14:14:47 | `^${pro ... r.app$` | provenance | | +| RegExpInjection.js:16:7:16:28 | argv | RegExpInjection.js:17:14:17:17 | argv | provenance | | +| RegExpInjection.js:16:14:16:25 | process.argv | RegExpInjection.js:16:7:16:28 | argv | provenance | | +| RegExpInjection.js:20:7:20:36 | userInput | RegExpInjection.js:21:14:21:22 | userInput | provenance | | +| RegExpInjection.js:20:19:20:36 | req.param("input") | RegExpInjection.js:20:7:20:36 | userInput | provenance | | +nodes +| RegExpInjection.js:6:14:6:48 | `^${pro ... r.app$` | semmle.label | `^${pro ... r.app$` | +| RegExpInjection.js:6:18:6:28 | process.env | semmle.label | process.env | +| RegExpInjection.js:8:14:8:40 | `^${pro ... }/bin$` | semmle.label | `^${pro ... }/bin$` | +| RegExpInjection.js:8:18:8:28 | process.env | semmle.label | process.env | +| RegExpInjection.js:10:7:10:35 | envVar | semmle.label | envVar | +| RegExpInjection.js:10:16:10:26 | process.env | semmle.label | process.env | +| RegExpInjection.js:11:14:11:19 | envVar | semmle.label | envVar | +| RegExpInjection.js:14:14:14:47 | `^${pro ... r.app$` | semmle.label | `^${pro ... r.app$` | +| RegExpInjection.js:14:18:14:29 | process.argv | semmle.label | process.argv | +| RegExpInjection.js:16:7:16:28 | argv | semmle.label | argv | +| RegExpInjection.js:16:14:16:25 | process.argv | semmle.label | process.argv | +| RegExpInjection.js:17:14:17:17 | argv | semmle.label | argv | +| RegExpInjection.js:20:7:20:36 | userInput | semmle.label | userInput | +| RegExpInjection.js:20:19:20:36 | req.param("input") | semmle.label | req.param("input") | +| RegExpInjection.js:21:14:21:22 | userInput | semmle.label | userInput | +subpaths diff --git a/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.ext.yml b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.ext.yml new file mode 100644 index 000000000000..cd28c6d97174 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/threat-models + extensible: threatModelConfiguration + data: + - ["environment", true, 0] diff --git a/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.js b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.js new file mode 100644 index 000000000000..28736678d8c3 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.js @@ -0,0 +1,22 @@ +var express = require('express'); +var app = express(); + +app.get('/test-environment', function(req, res) { + // Environment variables should be detected when "environment" threat model is enabled + new RegExp(`^${process.env.HOME}/Foo/bar.app$`); // $ Alert[js/regex-injection] + + new RegExp(`^${process.env.PATH}/bin$`); // $ Alert[js/regex-injection] + + var envVar = process.env.NODE_ENV; // $ Source[js/regex-injection] + new RegExp(envVar); // $ Alert[js/regex-injection] + + // Command line arguments should still be detected + new RegExp(`^${process.argv[1]}/Foo/bar.app$`); // $ Alert[js/regex-injection] + + var argv = process.argv[2]; // $ Source[js/regex-injection] + new RegExp(argv); // $ Alert[js/regex-injection] + + // Regular user input should still be detected + var userInput = req.param("input"); // $ Source[js/regex-injection] + new RegExp(userInput); // $ Alert[js/regex-injection] +}); diff --git a/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.qlref b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.qlref new file mode 100644 index 000000000000..2bf1a8eee365 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-730/Threat-models-enabled/RegExpInjection.qlref @@ -0,0 +1,2 @@ +query: Security/CWE-730/RegExpInjection.ql +postprocess: utils/test/InlineExpectationsTestQuery.ql