Skip to content

Commit 6279c67

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5901 from erik-krogh/regFP
Approved by asgerf
2 parents 025043a + 3766678 commit 6279c67

File tree

4 files changed

+77
-2
lines changed

4 files changed

+77
-2
lines changed

‎javascript/ql/src/semmle/javascript/Regexp.qll

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,8 +1237,18 @@ module RegExp {
12371237
}
12381238
}
12391239

1240-
private class DefaultMetaCharacter extends MetaCharacter {
1241-
DefaultMetaCharacter() { this = ["<", "'", "\""] }
1240+
/**
1241+
* A meta character used by HTML.
1242+
*/
1243+
private class HTMLMetaCharacter extends MetaCharacter {
1244+
HTMLMetaCharacter() { this = ["<", "'", "\""] }
1245+
}
1246+
1247+
/**
1248+
* A meta character used by regular expressions.
1249+
*/
1250+
private class RegexpMetaChars extends RegExp::MetaCharacter {
1251+
RegexpMetaChars() { this = ["{", "[", "+"] }
12421252
}
12431253

12441254
/**

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,4 +55,20 @@ module RegExpInjection {
5555
)
5656
}
5757
}
58+
59+
/**
60+
* A global regexp replacement involving the `{`, `[`, or `+` meta-character, viewed as a sanitizer for
61+
* regexp-injection vulnerabilities.
62+
*/
63+
class MetacharEscapeSanitizer extends Sanitizer, StringReplaceCall {
64+
MetacharEscapeSanitizer() {
65+
isGlobal() and
66+
(
67+
RegExp::alwaysMatchesMetaCharacter(getRegExp().getRoot(), ["{", "[", "+"])
68+
or
69+
// or it's like a wild-card.
70+
RegExp::isWildcardLike(getRegExp().getRoot())
71+
)
72+
}
73+
}
5874
}

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,18 @@ nodes
4444
| RegExpInjection.js:54:14:54:42 | key.spl ... x => x) |
4545
| RegExpInjection.js:54:14:54:52 | key.spl ... in("-") |
4646
| RegExpInjection.js:54:14:54:52 | key.spl ... in("-") |
47+
| RegExpInjection.js:60:31:60:56 | input |
48+
| RegExpInjection.js:60:39:60:56 | req.param("input") |
49+
| RegExpInjection.js:60:39:60:56 | req.param("input") |
50+
| RegExpInjection.js:64:14:64:18 | input |
51+
| RegExpInjection.js:64:14:64:18 | input |
52+
| RegExpInjection.js:82:7:82:32 | input |
53+
| RegExpInjection.js:82:15:82:32 | req.param("input") |
54+
| RegExpInjection.js:82:15:82:32 | req.param("input") |
55+
| RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" |
56+
| RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" |
57+
| RegExpInjection.js:87:25:87:29 | input |
58+
| RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") |
4759
| tst.js:1:46:1:46 | e |
4860
| tst.js:1:46:1:46 | e |
4961
| tst.js:2:9:2:21 | data |
@@ -99,6 +111,16 @@ edges
99111
| RegExpInjection.js:54:14:54:27 | key.split(".") | RegExpInjection.js:54:14:54:42 | key.spl ... x => x) |
100112
| RegExpInjection.js:54:14:54:42 | key.spl ... x => x) | RegExpInjection.js:54:14:54:52 | key.spl ... in("-") |
101113
| RegExpInjection.js:54:14:54:42 | key.spl ... x => x) | RegExpInjection.js:54:14:54:52 | key.spl ... in("-") |
114+
| RegExpInjection.js:60:31:60:56 | input | RegExpInjection.js:64:14:64:18 | input |
115+
| RegExpInjection.js:60:31:60:56 | input | RegExpInjection.js:64:14:64:18 | input |
116+
| RegExpInjection.js:60:39:60:56 | req.param("input") | RegExpInjection.js:60:31:60:56 | input |
117+
| RegExpInjection.js:60:39:60:56 | req.param("input") | RegExpInjection.js:60:31:60:56 | input |
118+
| RegExpInjection.js:82:7:82:32 | input | RegExpInjection.js:87:25:87:29 | input |
119+
| RegExpInjection.js:82:15:82:32 | req.param("input") | RegExpInjection.js:82:7:82:32 | input |
120+
| RegExpInjection.js:82:15:82:32 | req.param("input") | RegExpInjection.js:82:7:82:32 | input |
121+
| RegExpInjection.js:87:25:87:29 | input | RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") |
122+
| RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" |
123+
| RegExpInjection.js:87:25:87:48 | input.r ... g, "\|") | RegExpInjection.js:87:14:87:55 | "^.*\\.( ... + ")$" |
102124
| tst.js:1:46:1:46 | e | tst.js:2:16:2:16 | e |
103125
| tst.js:1:46:1:46 | e | tst.js:2:16:2:16 | e |
104126
| tst.js:2:9:2:21 | data | tst.js:3:21:3:24 | data |
@@ -122,4 +144,6 @@ edges
122144
| RegExpInjection.js:47:22:47:26 | input | RegExpInjection.js:5:39:5:56 | req.param("input") | RegExpInjection.js:47:22:47:26 | input | This regular expression is constructed from a $@. | RegExpInjection.js:5:39:5:56 | req.param("input") | user-provided value |
123145
| RegExpInjection.js:50:46:50:50 | input | RegExpInjection.js:5:39:5:56 | req.param("input") | RegExpInjection.js:50:46:50:50 | input | This regular expression is constructed from a $@. | RegExpInjection.js:5:39:5:56 | req.param("input") | user-provided value |
124146
| 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 |
147+
| 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 |
148+
| 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 |
125149
| 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: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,29 @@ app.get('/findKey', function(req, res) {
6060
var key = req.param("key"), input = req.param("input");
6161

6262
Search.search(input); // OK!
63+
64+
new RegExp(input); // NOT OK
65+
66+
var sanitized = input.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&");
67+
new RegExp(sanitized); // OK
68+
});
69+
70+
function escape1(pattern) {
71+
return pattern.replace(/[\x00-\x7f]/g,
72+
function(s) { return '\\x' + ('00' + s.charCodeAt().toString(16)).substr(-2); });
73+
}
74+
75+
function escape2(str){
76+
return str.replace(/([\.$?*|{}\(\)\[\]\\\/\+\-^])/g, function(ch){
77+
return "\\" + ch;
78+
});
79+
};
80+
81+
app.get('/has-sanitizer', function(req, res) {
82+
var input = req.param("input");
83+
84+
new RegExp(escape1(input)); // OK
85+
new RegExp(escape2(input)); // OK
86+
87+
new RegExp("^.*\.(" + input.replace(/,/g, "|") + ")$"); // NOT OK
6388
});

0 commit comments

Comments
 (0)