mirror of
https://github.com/github/codeql.git
synced 2026-04-30 11:15:13 +02:00
don't lowercase the endpointExample, and correctly handle root states
This commit is contained in:
@@ -24,7 +24,7 @@ import semmle.javascript.security.regexp.NfaUtils as NfaUtils
|
||||
|
||||
/** Holds if `s` is a relevant regexp term were we want to compute a string that matches the term (for `getCaseSensitiveBypassExample`). */
|
||||
predicate isCand(NfaUtils::State s) {
|
||||
s.getRepr().isRootTerm() and
|
||||
s.getRepr() instanceof NfaUtils::RegExpRoot and
|
||||
exists(DataFlow::RegExpCreationNode creation |
|
||||
isCaseSensitiveMiddleware(_, creation, _) and
|
||||
s.getRepr().getRootTerm() = creation.getRoot()
|
||||
@@ -87,13 +87,17 @@ predicate isGuardedCaseInsensitiveEndpoint(
|
||||
*/
|
||||
string getAnEndpointExample(Routing::RouteSetup endpoint) {
|
||||
exists(string raw |
|
||||
raw = endpoint.getRelativePath().toLowerCase() and
|
||||
raw = endpoint.getRelativePath() and
|
||||
result = raw.regexpReplaceAll(":\\w+\\b|\\*", ["a", "1"])
|
||||
)
|
||||
}
|
||||
|
||||
import semmle.javascript.security.regexp.RegexpMatching as RegexpMatching
|
||||
|
||||
NfaUtils::RegExpRoot getARoot(DataFlow::RegExpCreationNode creator) {
|
||||
result.getRootTerm() = creator.getRoot()
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if the regexp matcher should test whether `root` matches `str`.
|
||||
* The result is used to test whether a case-sensitive bypass exists.
|
||||
@@ -109,10 +113,10 @@ predicate isMatchingCandidate(
|
||||
isGuardedCaseInsensitiveEndpoint(endPoint, middleware)
|
||||
|
|
||||
root = regexp.getRoot() and
|
||||
exists(getCaseSensitiveBypassExample(root)) and
|
||||
exists(getCaseSensitiveBypassExample(getARoot(regexp))) and
|
||||
ignorePrefix = true and
|
||||
testWithGroups = false and
|
||||
str = [getCaseSensitiveBypassExample(root), getAnEndpointExample(endPoint)]
|
||||
str = [getCaseSensitiveBypassExample(getARoot(regexp)), getAnEndpointExample(endPoint)]
|
||||
)
|
||||
}
|
||||
|
||||
@@ -120,15 +124,14 @@ import RegexpMatching::RegexpMatching<isMatchingCandidate/4> as Matcher
|
||||
|
||||
from
|
||||
DataFlow::RegExpCreationNode regexp, Routing::RouteSetup middleware, Routing::RouteSetup endpoint,
|
||||
DataFlow::Node arg, string byPassExample
|
||||
DataFlow::Node arg, string byPassExample, string endpointExample
|
||||
where
|
||||
isCaseSensitiveMiddleware(middleware, regexp, arg) and
|
||||
byPassExample = getCaseSensitiveBypassExample(regexp.getRoot()) and
|
||||
byPassExample = getCaseSensitiveBypassExample(getARoot(regexp)) and
|
||||
isGuardedCaseInsensitiveEndpoint(endpoint, middleware) and
|
||||
exists(string endpointExample | endpointExample = getAnEndpointExample(endpoint) |
|
||||
Matcher::matches(regexp.getRoot(), endpointExample) and
|
||||
not Matcher::matches(regexp.getRoot(), byPassExample)
|
||||
)
|
||||
endpointExample = getAnEndpointExample(endpoint) and
|
||||
Matcher::matches(regexp.getRoot(), endpointExample) and
|
||||
not Matcher::matches(regexp.getRoot(), byPassExample)
|
||||
select arg,
|
||||
"This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '"
|
||||
+ byPassExample + "' will bypass the middleware.", regexp, "pattern", endpoint, "here"
|
||||
|
||||
@@ -4,3 +4,5 @@
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| tst.js:91:9:91:40 | /\\/summ ... ntGame/ | This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '/CURRENTGAME' will bypass the middleware. | tst.js:91:9:91:40 | /\\/summ ... ntGame/ | pattern | tst.js:93:1:95:2 | app.get ... O");\\n}) | here |
|
||||
| tst.js:91:9:91:40 | /\\/summ ... ntGame/ | This route uses a case-sensitive path $@, but is guarding a case-insensitive path $@. A path such as '/SUMMONERBYNAME' will bypass the middleware. | tst.js:91:9:91:40 | /\\/summ ... ntGame/ | pattern | tst.js:93:1:95:2 | app.get ... O");\\n}) | here |
|
||||
|
||||
@@ -86,4 +86,10 @@ app.get('/baz2/a', (req, resp) => {
|
||||
app.use(/\/[Bb][Aa][Zz]3\/[a]/, unknown()); // NOT OK - case sensitive
|
||||
app.get('/baz3/a', (req, resp) => {
|
||||
resp.send({ test: 123 });
|
||||
});
|
||||
});
|
||||
|
||||
app.use(/\/summonerByName|\/currentGame/,apiLimit1, apiLimit2);
|
||||
|
||||
app.get('/currentGame', function (req, res) {
|
||||
res.send("FOO");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user