From e38b63ebcda88f698ab728c8d2aabd12cef06c51 Mon Sep 17 00:00:00 2001 From: Napalys Date: Mon, 25 Nov 2024 11:56:06 +0100 Subject: [PATCH] JS: previously js/case-sensitive-middleware-path was not taking into consideration unknown flags --- .../ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql | 2 +- .../Security/CWE-178/CaseSensitiveMiddlewarePath.expected | 1 + javascript/ql/test/query-tests/Security/CWE-178/tst.js | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql b/javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql index d16f72d4172a..2045d801c702 100644 --- a/javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql +++ b/javascript/ql/src/Security/CWE-178/CaseSensitiveMiddlewarePath.ql @@ -65,7 +65,7 @@ predicate isCaseSensitiveMiddleware( arg = call.getArgument(0) and regexp.getAReference().flowsTo(arg) and exists(string flags | - flags = regexp.getFlags() and + flags = regexp.tryGetFlags() and not RegExp::isIgnoreCase(flags) ) ) diff --git a/javascript/ql/test/query-tests/Security/CWE-178/CaseSensitiveMiddlewarePath.expected b/javascript/ql/test/query-tests/Security/CWE-178/CaseSensitiveMiddlewarePath.expected index 2be1780188a0..cb1720260ef8 100644 --- a/javascript/ql/test/query-tests/Security/CWE-178/CaseSensitiveMiddlewarePath.expected +++ b/javascript/ql/test/query-tests/Security/CWE-178/CaseSensitiveMiddlewarePath.expected @@ -6,3 +6,4 @@ | tst.js:76:9:76:20 | /\\/baz\\/bla/ | This route uses a case-sensitive path $@, but is guarding a $@. 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}) | case-insensitive path | | tst.js:86:9:86:30 | /\\/[Bb] ... 3\\/[a]/ | This route uses a case-sensitive path $@, but is guarding a $@. 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}) | case-insensitive path | | tst.js:91:9:91:40 | /\\/summ ... ntGame/ | This route uses a case-sensitive path $@, but is guarding a $@. 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}) | case-insensitive path | +| tst.js:98:5:98:43 | new Reg ... Flag()) | This route uses a case-sensitive path $@, but is guarding a $@. A path such as '/BAR/1' will bypass the middleware. | tst.js:98:5:98:43 | new Reg ... Flag()) | pattern | tst.js:107:1:108:2 | app.get ... ware\\n}) | case-insensitive path | diff --git a/javascript/ql/test/query-tests/Security/CWE-178/tst.js b/javascript/ql/test/query-tests/Security/CWE-178/tst.js index 3dd24029ebe0..69a203cb8df0 100644 --- a/javascript/ql/test/query-tests/Security/CWE-178/tst.js +++ b/javascript/ql/test/query-tests/Security/CWE-178/tst.js @@ -95,7 +95,7 @@ app.get('/currentGame', function (req, res) { }); app.get( - new RegExp('^/bar(.*)?', unknownFlag()), // NOT OK - Currently not flagged. + new RegExp('^/bar(.*)?', unknownFlag()), // NOT OK - Might be OK if the unknown flag evaluates to case insensitive one unknown(), function(req, res, next) { if (req.params.blah) {