diff --git a/javascript/ql/lib/change-notes/2023-02-14-regex-injection-process-env.md b/javascript/ql/lib/change-notes/2023-02-14-regex-injection-process-env.md new file mode 100644 index 00000000000..504b71a92b5 --- /dev/null +++ b/javascript/ql/lib/change-notes/2023-02-14-regex-injection-process-env.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `js/regex-injection` query now recognizes environment variables and command-line arguments as sources. \ No newline at end of file diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll index eed346d8d0b..14fbcd4f400 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll @@ -10,7 +10,10 @@ module IndirectCommandInjection { /** * A data flow source for command-injection vulnerabilities. */ - abstract class Source extends DataFlow::Node { } + abstract class Source extends DataFlow::Node { + /** Gets a description of this source. */ + string describe() { result = "command-line argument" } + } /** * A data flow sink for command-injection vulnerabilities. @@ -37,6 +40,15 @@ module IndirectCommandInjection { } } + /** + * A read of `process.env`, considered as a flow source for command injection. + */ + private class ProcessEnvAsSource extends Source { + ProcessEnvAsSource() { this = NodeJSLib::process().getAPropertyRead("env") } + + override string describe() { result = "environment variable" } + } + /** * An object containing parsed command-line arguments, considered as a flow source for command injection. */ diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll index ab9f3d90e02..6f2499c7fe4 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll @@ -10,7 +10,10 @@ module RegExpInjection { /** * A data flow source for untrusted user input used to construct regular expressions. */ - abstract class Source extends DataFlow::Node { } + abstract class Source extends DataFlow::Node { + /** Gets a description of this source. */ + string describe() { result = "user-provided value" } + } /** * A data flow sink for untrusted user input used to construct regular expressions. @@ -30,6 +33,16 @@ module RegExpInjection { RemoteFlowSourceAsSource() { not this instanceof ClientSideRemoteFlowSource } } + private import IndirectCommandInjectionCustomizations + + /** + * A read of `process.env`, `process.argv`, and similar, considered as a flow source for regular + * expression injection. + */ + class ArgvAsSource extends Source instanceof IndirectCommandInjection::Source { + override string describe() { result = IndirectCommandInjection::Source.super.describe() } + } + /** * The source string of a regular expression. */ diff --git a/javascript/ql/src/Security/CWE-078/IndirectCommandInjection.ql b/javascript/ql/src/Security/CWE-078/IndirectCommandInjection.ql index 7520a95ed9c..34f89023441 100644 --- a/javascript/ql/src/Security/CWE-078/IndirectCommandInjection.ql +++ b/javascript/ql/src/Security/CWE-078/IndirectCommandInjection.ql @@ -25,4 +25,4 @@ where then cfg.isSinkWithHighlight(sink.getNode(), highlight) else highlight = sink.getNode() select highlight, source, sink, "This command depends on an unsanitized $@.", source.getNode(), - "command-line argument" + source.getNode().(Source).describe() diff --git a/javascript/ql/src/Security/CWE-730/RegExpInjection.ql b/javascript/ql/src/Security/CWE-730/RegExpInjection.ql index 8da2080d167..5b679cf1dcf 100644 --- a/javascript/ql/src/Security/CWE-730/RegExpInjection.ql +++ b/javascript/ql/src/Security/CWE-730/RegExpInjection.ql @@ -20,4 +20,4 @@ import DataFlow::PathGraph from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink where cfg.hasFlowPath(source, sink) select sink.getNode(), source, sink, "This regular expression is constructed from a $@.", - source.getNode(), "user-provided value" + source.getNode(), source.getNode().(Source).describe() diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected index 7d24e94a590..693ef9e5e95 100644 --- a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected @@ -56,6 +56,16 @@ nodes | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" | | RegExpInjection.js:87:25:87:29 | input | | RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | +| RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` | +| RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` | +| RegExpInjection.js:91:20:91:30 | process.env | +| RegExpInjection.js:91:20:91:30 | process.env | +| RegExpInjection.js:91:20:91:35 | process.env.HOME | +| RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` | +| RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` | +| RegExpInjection.js:93:20:93:31 | process.argv | +| RegExpInjection.js:93:20:93:31 | process.argv | +| RegExpInjection.js:93:20:93:34 | process.argv[1] | | tst.js:1:46:1:46 | e | | tst.js:1:46:1:46 | e | | tst.js:2:9:2:21 | data | @@ -121,6 +131,14 @@ edges | RegExpInjection.js:87:25:87:29 | input | RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | | RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" | | RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" | +| RegExpInjection.js:91:20:91:30 | process.env | RegExpInjection.js:91:20:91:35 | process.env.HOME | +| RegExpInjection.js:91:20:91:30 | process.env | RegExpInjection.js:91:20:91:35 | process.env.HOME | +| RegExpInjection.js:91:20:91:35 | process.env.HOME | RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` | +| RegExpInjection.js:91:20:91:35 | process.env.HOME | RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` | +| RegExpInjection.js:93:20:93:31 | process.argv | RegExpInjection.js:93:20:93:34 | process.argv[1] | +| RegExpInjection.js:93:20:93:31 | process.argv | RegExpInjection.js:93:20:93:34 | process.argv[1] | +| RegExpInjection.js:93:20:93:34 | process.argv[1] | RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` | +| RegExpInjection.js:93:20:93:34 | process.argv[1] | RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` | | tst.js:1:46:1:46 | e | tst.js:2:16:2:16 | e | | tst.js:1:46:1:46 | e | tst.js:2:16:2:16 | e | | tst.js:2:9:2:21 | data | tst.js:3:21:3:24 | data | @@ -146,4 +164,6 @@ edges | RegExpInjection.js:54:14:54:52 | key.spl ... in("-") | RegExpInjection.js:5:13:5:28 | req.param("key") | RegExpInjection.js:54:14:54: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:64:14:64:18 | input | RegExpInjection.js:60:39:60:56 | req.param("input") | RegExpInjection.js:64:14:64:18 | input | This regular expression is constructed from a $@. | RegExpInjection.js:60:39:60:56 | req.param("input") | user-provided value | | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" | RegExpInjection.js:82:15:82:32 | req.param("input") | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" | This regular expression is constructed from a $@. | RegExpInjection.js:82:15:82:32 | req.param("input") | user-provided value | +| RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` | RegExpInjection.js:91:20:91:30 | process.env | RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` | This regular expression is constructed from a $@. | RegExpInjection.js:91:20:91:30 | process.env | environment variable | +| RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` | RegExpInjection.js:93:20:93:31 | process.argv | RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` | This regular expression is constructed from a $@. | RegExpInjection.js:93:20:93:31 | process.argv | command-line argument | | tst.js:3:16:3:35 | "^"+ data.name + "$" | tst.js:1:46:1:46 | e | tst.js:3:16:3:35 | "^"+ data.name + "$" | This regular expression is constructed from a $@. | tst.js:1:46:1:46 | e | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js index 3551a1ff72a..1f8113f7d75 100644 --- a/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js +++ b/javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js @@ -86,3 +86,9 @@ app.get('/has-sanitizer', function(req, res) { new RegExp("^.*\.(" + input.replace(/,/g, "|") + ")$"); // NOT OK }); + +app.get("argv", function(req, res) { + new RegExp(`^${process.env.HOME}/Foo/bar.app$`); // NOT OK + + new RegExp(`^${process.argv[1]}/Foo/bar.app$`); // NOT OK +});