Skip to content

Commit 26fcf6b

Browse files
committed
apply suggestions from review
1 parent de3e1c3 commit 26fcf6b

File tree

3 files changed

+17
-3
lines changed

3 files changed

+17
-3
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,13 +104,13 @@ predicate isGuardedCaseInsensitiveEndpoint(
104104
}
105105

106106
/**
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.
107+
* Gets an example path that will hit `endpoint`.
108+
* Query parameters (e.g. the ":param" in "/foo/:param") have been replaced with example values.
109109
*/
110110
string getAnEndpointExample(Routing::RouteSetup endpoint) {
111111
exists(string raw |
112112
raw = endpoint.getRelativePath().toLowerCase() and
113-
result = raw.regexpReplaceAll(":\\w+\\b", ["a", "1"])
113+
result = raw.regexpReplaceAll(":\\w+\\b|\\*", ["a", "1"])
114114
)
115115
}
116116

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
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 |
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 |
4+
| 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 |

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,3 +59,16 @@ app.use(/\/foo\/([0-9]+)/i, (req, res) => { // OK - not middleware (also case in
5959

6060
app.get('/foo/:param', (req, res) => { // OK - not a middleware
6161
});
62+
63+
app.get(
64+
new RegExp('^/bar(.*)?'), // NOT OK - case sensitive
65+
unknown(),
66+
function(req, res, next) {
67+
if (req.params.blah) {
68+
next();
69+
}
70+
}
71+
);
72+
73+
app.get('/bar/*', (req, res) => { // OK - not a middleware
74+
});

0 commit comments

Comments
 (0)