Skip to content

Commit f8f926a

Browse files
erik-krogherik-krogh
authored
Merge pull request github#12175 from erik-krogh/reg-input
JS: add process.env and process.argv etc. as source for `js/regex-injection`
2 parents 4ffe20a + de4f501 commit f8f926a

File tree

7 files changed

+59
-4
lines changed

7 files changed

+59
-4
lines changed

‎javascript/ql/lib/change-notes/2023-02-14-regex-injection-process-env.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `js/regex-injection` query now recognizes environment variables and command-line arguments as sources.

‎javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ module IndirectCommandInjection {
1010
/**
1111
* A data flow source for command-injection vulnerabilities.
1212
*/
13-
abstract class Source extends DataFlow::Node { }
13+
abstract class Source extends DataFlow::Node {
14+
/** Gets a description of this source. */
15+
string describe() { result = "command-line argument" }
16+
}
1417

1518
/**
1619
* A data flow sink for command-injection vulnerabilities.
@@ -37,6 +40,15 @@ module IndirectCommandInjection {
3740
}
3841
}
3942

43+
/**
44+
* A read of `process.env`, considered as a flow source for command injection.
45+
*/
46+
private class ProcessEnvAsSource extends Source {
47+
ProcessEnvAsSource() { this = NodeJSLib::process().getAPropertyRead("env") }
48+
49+
override string describe() { result = "environment variable" }
50+
}
51+
4052
/**
4153
* An object containing parsed command-line arguments, considered as a flow source for command injection.
4254
*/

‎javascript/ql/lib/semmle/javascript/security/dataflow/RegExpInjectionCustomizations.qll

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@ module RegExpInjection {
1010
/**
1111
* A data flow source for untrusted user input used to construct regular expressions.
1212
*/
13-
abstract class Source extends DataFlow::Node { }
13+
abstract class Source extends DataFlow::Node {
14+
/** Gets a description of this source. */
15+
string describe() { result = "user-provided value" }
16+
}
1417

1518
/**
1619
* A data flow sink for untrusted user input used to construct regular expressions.
@@ -30,6 +33,16 @@ module RegExpInjection {
3033
RemoteFlowSourceAsSource() { not this instanceof ClientSideRemoteFlowSource }
3134
}
3235

36+
private import IndirectCommandInjectionCustomizations
37+
38+
/**
39+
* A read of `process.env`, `process.argv`, and similar, considered as a flow source for regular
40+
* expression injection.
41+
*/
42+
class ArgvAsSource extends Source instanceof IndirectCommandInjection::Source {
43+
override string describe() { result = IndirectCommandInjection::Source.super.describe() }
44+
}
45+
3346
/**
3447
* The source string of a regular expression.
3548
*/

‎javascript/ql/src/Security/CWE-078/IndirectCommandInjection.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,4 +25,4 @@ where
2525
then cfg.isSinkWithHighlight(sink.getNode(), highlight)
2626
else highlight = sink.getNode()
2727
select highlight, source, sink, "This command depends on an unsanitized $@.", source.getNode(),
28-
"command-line argument"
28+
source.getNode().(Source).describe()

‎javascript/ql/src/Security/CWE-730/RegExpInjection.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ import DataFlow::PathGraph
2020
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
2121
where cfg.hasFlowPath(source, sink)
2222
select sink.getNode(), source, sink, "This regular expression is constructed from a $@.",
23-
source.getNode(), "user-provided value"
23+
source.getNode(), source.getNode().(Source).describe()

‎javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.expected

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,16 @@ nodes
5656
| RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" |
5757
| RegExpInjection.js:87:25:87:29 | input |
5858
| RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") |
59+
| RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` |
60+
| RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` |
61+
| RegExpInjection.js:91:20:91:30 | process.env |
62+
| RegExpInjection.js:91:20:91:30 | process.env |
63+
| RegExpInjection.js:91:20:91:35 | process.env.HOME |
64+
| RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` |
65+
| RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` |
66+
| RegExpInjection.js:93:20:93:31 | process.argv |
67+
| RegExpInjection.js:93:20:93:31 | process.argv |
68+
| RegExpInjection.js:93:20:93:34 | process.argv[1] |
5969
| tst.js:1:46:1:46 | e |
6070
| tst.js:1:46:1:46 | e |
6171
| tst.js:2:9:2:21 | data |
@@ -121,6 +131,14 @@ edges
121131
| RegExpInjection.js:87:25:87:29 | input | RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") |
122132
| RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" |
123133
| RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" |
134+
| RegExpInjection.js:91:20:91:30 | process.env | RegExpInjection.js:91:20:91:35 | process.env.HOME |
135+
| RegExpInjection.js:91:20:91:30 | process.env | RegExpInjection.js:91:20:91:35 | process.env.HOME |
136+
| RegExpInjection.js:91:20:91:35 | process.env.HOME | RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` |
137+
| RegExpInjection.js:91:20:91:35 | process.env.HOME | RegExpInjection.js:91:16:91:50 | `^${pro ... r.app$` |
138+
| RegExpInjection.js:93:20:93:31 | process.argv | RegExpInjection.js:93:20:93:34 | process.argv[1] |
139+
| RegExpInjection.js:93:20:93:31 | process.argv | RegExpInjection.js:93:20:93:34 | process.argv[1] |
140+
| RegExpInjection.js:93:20:93:34 | process.argv[1] | RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` |
141+
| RegExpInjection.js:93:20:93:34 | process.argv[1] | RegExpInjection.js:93:16:93:49 | `^${pro ... r.app$` |
124142
| tst.js:1:46:1:46 | e | tst.js:2:16:2:16 | e |
125143
| tst.js:1:46:1:46 | e | tst.js:2:16:2:16 | e |
126144
| tst.js:2:9:2:21 | data | tst.js:3:21:3:24 | data |
@@ -146,4 +164,6 @@ edges
146164
| 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 |
147165
| 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 |
148166
| 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 |
167+
| 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 |
168+
| 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 |
149169
| 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 |

‎javascript/ql/test/query-tests/Security/CWE-730/RegExpInjection.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,9 @@ app.get('/has-sanitizer', function(req, res) {
8686

8787
new RegExp("^.*\.(" + input.replace(/,/g, "|") + ")$"); // NOT OK
8888
});
89+
90+
app.get("argv", function(req, res) {
91+
new RegExp(`^${process.env.HOME}/Foo/bar.app$`); // NOT OK
92+
93+
new RegExp(`^${process.argv[1]}/Foo/bar.app$`); // NOT OK
94+
});

0 commit comments

Comments
 (0)