Skip to content

Commit de3e1c3

Browse files
committed
use the shared regular expression libraries in js/case-sensitive-middleware-path
1 parent 473bc92 commit de3e1c3

File tree

2 files changed

+64
-29
lines changed

2 files changed

+64
-29
lines changed

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

Lines changed: 63 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -41,34 +41,29 @@ predicate isLikelyCaseSensitiveRegExp(RegExpTerm term) {
4141
)
4242
}
4343

44-
/**
45-
* Gets a string matched by `term`, or part of such a string.
46-
*/
47-
string getExampleString(RegExpTerm term) {
48-
result = term.getAMatchedString()
49-
or
50-
// getAMatchedString does not recurse into sequences. Perform one step manually.
51-
exists(RegExpSequence seq | seq = term |
52-
result =
53-
strictconcat(RegExpTerm child, int i, string text |
54-
child = seq.getChild(i) and
55-
(
56-
text = child.getAMatchedString()
57-
or
58-
not exists(child.getAMatchedString()) and
59-
text = ""
60-
)
61-
|
62-
text order by i
63-
)
44+
import semmle.javascript.security.regexp.NfaUtils as NfaUtils
45+
46+
/** Holds if `s` is a relevant regexp term were we want to compute a string that matches the term (for `getCaseSensitiveBypassExample`). */
47+
predicate isCand(NfaUtils::State s) {
48+
s.getRepr().isRootTerm() and
49+
exists(DataFlow::RegExpCreationNode creation |
50+
isCaseSensitiveMiddleware(_, creation, _) and
51+
s.getRepr().getRootTerm() = creation.getRoot()
6452
)
6553
}
6654

55+
import NfaUtils::PrefixConstruction<isCand/1> as Prefix
56+
57+
/** Gets a string matched by `term`. */
58+
string getExampleString(RegExpTerm term) {
59+
result = Prefix::prefix(any(NfaUtils::State s | s.getRepr() = term))
60+
}
61+
6762
string getCaseSensitiveBypassExample(RegExpTerm term) {
68-
exists(string example |
69-
example = getExampleString(term) and
70-
result = toOtherCase(example) and
71-
result != example // getting an example string is approximate; ensure we got a proper case-change example
63+
exists(string byPassExample |
64+
byPassExample = getExampleString(term) and
65+
result = toOtherCase(byPassExample) and
66+
result != byPassExample // getting an byPassExample string is approximate; ensure we got a proper case-change byPassExample
7267
)
7368
}
7469

@@ -108,14 +103,54 @@ predicate isGuardedCaseInsensitiveEndpoint(
108103
)
109104
}
110105

106+
/**
107+
* Gets an byPassExample path that will hit `endpoint`.
108+
* Query parameters (e.g. the ":param" in "/foo/:param") have been replaced with byPassExample values.
109+
*/
110+
string getAnEndpointExample(Routing::RouteSetup endpoint) {
111+
exists(string raw |
112+
raw = endpoint.getRelativePath().toLowerCase() and
113+
result = raw.regexpReplaceAll(":\\w+\\b", ["a", "1"])
114+
)
115+
}
116+
117+
import semmle.javascript.security.regexp.RegexpMatching as RegexpMatching
118+
119+
/**
120+
* Holds if the regexp matcher should test whether `root` matches `str`.
121+
* The result is used to test whether a case-sensitive bypass exists.
122+
*/
123+
predicate isMatchingCandidate(
124+
RegexpMatching::RootTerm root, string str, boolean ignorePrefix, boolean testWithGroups
125+
) {
126+
exists(
127+
Routing::RouteSetup middleware, Routing::RouteSetup endPoint,
128+
DataFlow::RegExpCreationNode regexp
129+
|
130+
isCaseSensitiveMiddleware(middleware, regexp, _) and
131+
isGuardedCaseInsensitiveEndpoint(endPoint, middleware)
132+
|
133+
root = regexp.getRoot() and
134+
exists(getCaseSensitiveBypassExample(root)) and
135+
ignorePrefix = true and
136+
testWithGroups = false and
137+
str = [getCaseSensitiveBypassExample(root), getAnEndpointExample(endPoint)]
138+
)
139+
}
140+
141+
import RegexpMatching::RegexpMatching<isMatchingCandidate/4> as Matcher
142+
111143
from
112144
DataFlow::RegExpCreationNode regexp, Routing::RouteSetup middleware, Routing::RouteSetup endpoint,
113-
DataFlow::Node arg, string example
145+
DataFlow::Node arg, string byPassExample
114146
where
115147
isCaseSensitiveMiddleware(middleware, regexp, arg) and
116-
example = getCaseSensitiveBypassExample(regexp.getRoot()) and
148+
byPassExample = getCaseSensitiveBypassExample(regexp.getRoot()) and
117149
isGuardedCaseInsensitiveEndpoint(endpoint, middleware) and
118-
exists(endpoint.getRelativePath().toLowerCase().indexOf(example.toLowerCase()))
150+
exists(string endpointExample | endpointExample = getAnEndpointExample(endpoint) |
151+
Matcher::matches(regexp.getRoot(), endpointExample) and
152+
not Matcher::matches(regexp.getRoot(), byPassExample)
153+
)
119154
select arg,
120155
"This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '"
121-
+ example + "' will bypass the middleware.", regexp, "pattern", endpoint, "here"
156+
+ byPassExample + "' will bypass the middleware.", regexp, "pattern", endpoint, "here"
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
| tst.js:8:9:8:19 | /\\/foo\\/.*/ | 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:8:9:8:19 | /\\/foo\\/.*/ | pattern | tst.js:60:1:61:2 | app.get ... ware\\n}) | here |
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 |
3-
| 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/' 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 |
3+
| 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 |

0 commit comments

Comments
 (0)