Skip to content

JS: Exclude environment variables from js/regex-injection query by default #20148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import javascript
private import codeql.threatmodels.ThreatModels

module RegExpInjection {
/**
Expand Down Expand Up @@ -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" }
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -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 | |
Expand Down Expand Up @@ -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 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]
});
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
extensions:
- addsTo:
pack: codeql/threat-models
extensible: threatModelConfiguration
data:
- ["environment", true, 0]
Original file line number Diff line number Diff line change
@@ -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]
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: Security/CWE-730/RegExpInjection.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql