Skip to content

Commit d052b1e

Browse files
committed
also support regular expressions without repetitions
1 parent 26fcf6b commit d052b1e

File tree

7 files changed

+17
-30
lines changed

7 files changed

+17
-30
lines changed

java/ql/lib/semmle/code/java/security/regexp/NfaUtils.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,6 @@ class RegExpRoot extends RegExpTerm {
9393
* Holds if this root term is relevant to the ReDoS analysis.
9494
*/
9595
predicate isRelevant() {
96-
// there is at least one repetition
97-
getRoot(any(InfiniteRepetitionQuantifier q)) = this and
9896
// is actually used as a RegExp
9997
this.isUsedAsRegExp() and
10098
// not excluded for library specific reasons

javascript/ql/lib/semmle/javascript/security/regexp/NfaUtils.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,6 @@ class RegExpRoot extends RegExpTerm {
9393
* Holds if this root term is relevant to the ReDoS analysis.
9494
*/
9595
predicate isRelevant() {
96-
// there is at least one repetition
97-
getRoot(any(InfiniteRepetitionQuantifier q)) = this and
9896
// is actually used as a RegExp
9997
this.isUsedAsRegExp() and
10098
// not excluded for library specific reasons

javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,6 @@ string toOtherCase(string s) {
2020
if s.regexpMatch(".*[a-z].*") then result = s.toUpperCase() else result = s.toLowerCase()
2121
}
2222

23-
RegExpCharacterClass getEnclosingClass(RegExpTerm term) {
24-
term = result.getAChild()
25-
or
26-
term = result.getAChild().(RegExpRange).getAChild()
27-
}
28-
29-
/**
30-
* Holds if `term` seems to distinguish between upper and lower case letters, assuming the `i` flag is not present.
31-
*/
32-
pragma[inline]
33-
predicate isLikelyCaseSensitiveRegExp(RegExpTerm term) {
34-
exists(RegExpConstant const |
35-
const = term.getAChild*() and
36-
const.getValue().regexpMatch(".*[a-zA-Z].*") and
37-
not getEnclosingClass(const).getAChild().(RegExpConstant).getValue() =
38-
toOtherCase(const.getValue()) and
39-
not const.getParent*() instanceof RegExpNegativeLookahead and
40-
not const.getParent*() instanceof RegExpNegativeLookbehind
41-
)
42-
}
43-
4423
import semmle.javascript.security.regexp.NfaUtils as NfaUtils
4524

4625
/** Holds if `s` is a relevant regexp term were we want to compute a string that matches the term (for `getCaseSensitiveBypassExample`). */
@@ -84,7 +63,6 @@ predicate isCaseSensitiveMiddleware(
8463
) and
8564
arg = call.getArgument(0) and
8665
regexp.getAReference().flowsTo(arg) and
87-
isLikelyCaseSensitiveRegExp(regexp.getRoot()) and
8866
exists(string flags |
8967
flags = regexp.getFlags() and
9068
not RegExp::isIgnoreCase(flags)

javascript/ql/test/query-tests/Security/CWE-178/CaseSensitiveMiddlewarePath.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,5 @@
22
| tst.js:14:5:14:28 | new Reg ... (.*)?') | This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '/FOO' will bypass the middleware. | tst.js:14:5:14:28 | new Reg ... (.*)?') | pattern | tst.js:60:1:61:2 | app.get ... ware\\n}) | here |
33
| tst.js:41:9:41:25 | /\\/foo\\/([0-9]+)/ | This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '/FOO/0' will bypass the middleware. | tst.js:41:9:41:25 | /\\/foo\\/([0-9]+)/ | pattern | tst.js:60:1:61:2 | app.get ... ware\\n}) | here |
44
| tst.js:64:5:64:28 | new Reg ... (.*)?') | This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '/BAR' will bypass the middleware. | tst.js:64:5:64:28 | new Reg ... (.*)?') | pattern | tst.js:73:1:74:2 | app.get ... ware\\n}) | here |
5+
| tst.js:76:9:76:20 | /\\/baz\\/bla/ | This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '/BAZ/BLA' will bypass the middleware. | tst.js:76:9:76:20 | /\\/baz\\/bla/ | pattern | tst.js:77:1:79:2 | app.get ... });\\n}) | here |
6+
| tst.js:86:9:86:30 | /\\/[Bb] ... 3\\/[a]/ | This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '/BAZ3/A' will bypass the middleware. | tst.js:86:9:86:30 | /\\/[Bb] ... 3\\/[a]/ | pattern | tst.js:87:1:89:2 | app.get ... });\\n}) | here |

javascript/ql/test/query-tests/Security/CWE-178/tst.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,18 @@ app.get(
7272

7373
app.get('/bar/*', (req, res) => { // OK - not a middleware
7474
});
75+
76+
app.use(/\/baz\/bla/, unknown()); // NOT OK - case sensitive
77+
app.get('/baz/bla', (req, resp) => {
78+
resp.send({ test: 123 });
79+
});
80+
81+
app.use(/\/[Bb][Aa][Zz]2\/[aA]/, unknown()); // OK - not case sensitive
82+
app.get('/baz2/a', (req, resp) => {
83+
resp.send({ test: 123 });
84+
});
85+
86+
app.use(/\/[Bb][Aa][Zz]3\/[a]/, unknown()); // NOT OK - case sensitive
87+
app.get('/baz3/a', (req, resp) => {
88+
resp.send({ test: 123 });
89+
});

python/ql/lib/semmle/python/security/regexp/NfaUtils.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,6 @@ class RegExpRoot extends RegExpTerm {
9393
* Holds if this root term is relevant to the ReDoS analysis.
9494
*/
9595
predicate isRelevant() {
96-
// there is at least one repetition
97-
getRoot(any(InfiniteRepetitionQuantifier q)) = this and
9896
// is actually used as a RegExp
9997
this.isUsedAsRegExp() and
10098
// not excluded for library specific reasons

ruby/ql/lib/codeql/ruby/security/regexp/NfaUtils.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,6 @@ class RegExpRoot extends RegExpTerm {
9393
* Holds if this root term is relevant to the ReDoS analysis.
9494
*/
9595
predicate isRelevant() {
96-
// there is at least one repetition
97-
getRoot(any(InfiniteRepetitionQuantifier q)) = this and
9896
// is actually used as a RegExp
9997
this.isUsedAsRegExp() and
10098
// not excluded for library specific reasons

0 commit comments

Comments
 (0)