diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll index 157325891c8d..5f4ad1b3d73e 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll @@ -114,7 +114,13 @@ abstract class RateLimitingMiddleware extends DataFlow::SourceNode { * A rate limiter constructed using the `express-rate-limit` package. */ class ExpressRateLimit extends RateLimitingMiddleware { - ExpressRateLimit() { this = API::moduleImport("express-rate-limit").getReturn().asSource() } + ExpressRateLimit() { + this = + [ + API::moduleImport("express-rate-limit"), + API::moduleImport("express-rate-limit").getMember("rateLimit") + ].getReturn().asSource() + } } /** diff --git a/javascript/ql/src/change-notes/2023-10-26-express-rate-limit.md b/javascript/ql/src/change-notes/2023-10-26-express-rate-limit.md new file mode 100644 index 000000000000..28804e979083 --- /dev/null +++ b/javascript/ql/src/change-notes/2023-10-26-express-rate-limit.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Added modeling for importing `express-rate-limit` using a named import. diff --git a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected index b4b09f091da1..3d7bc2954eba 100644 --- a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected +++ b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/MissingRateLimiting.expected @@ -1,3 +1,11 @@ | MissingRateLimiting.js:4:19:8:1 | functio ... ath);\\n} | This route handler performs $@, but is not rate-limited. | MissingRateLimiting.js:7:5:7:22 | res.sendFile(path) | a file system access | | MissingRateLimiting.js:25:19:25:20 | f1 | This route handler performs $@, but is not rate-limited. | MissingRateLimiting.js:13:5:13:22 | res.sendFile(path) | a file system access | | MissingRateLimiting.js:25:27:25:28 | f3 | This route handler performs $@, but is not rate-limited. | MissingRateLimiting.js:22:5:22:22 | res.sendFile(path) | a file system access | +| tst.js:22:24:22:40 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization | +| tst.js:35:20:35:36 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization | +| tst.js:36:20:36:36 | expensiveHandler2 | This route handler performs $@, but is not rate-limited. | tst.js:15:40:15:73 | fs.writ ... quest") | a file system access | +| tst.js:37:20:37:36 | expensiveHandler3 | This route handler performs $@, but is not rate-limited. | tst.js:16:40:16:70 | child_p ... /true") | a system command | +| tst.js:38:20:38:36 | expensiveHandler4 | This route handler performs $@, but is not rate-limited. | tst.js:17:40:17:83 | connect ... ution') | a database access | +| tst.js:64:25:64:63 | functio ... req); } | This route handler performs $@, but is not rate-limited. | tst.js:64:46:64:60 | verifyUser(req) | authorization | +| tst.js:76:25:76:53 | catchAs ... ndler1) | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization | +| tst.js:88:24:88:40 | expensiveHandler1 | This route handler performs $@, but is not rate-limited. | tst.js:14:40:14:46 | login() | authorization | diff --git a/javascript/ql/test/query-tests/Security/CWE-770/tst.js b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-770/tst.js rename to javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.js diff --git a/javascript/ql/test/query-tests/Security/CWE-770/tst2.ts b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst2.ts similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-770/tst2.ts rename to javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst2.ts diff --git a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst3.js b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst3.js new file mode 100644 index 000000000000..b1bbc538ed95 --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst3.js @@ -0,0 +1,10 @@ +import express from "express"; +import { rateLimit } from "express-rate-limit"; + +const app = express(); + +const limiter = rateLimit(); +app.use(limiter) + +function expensiveHandler(req, res) { login(); } +app.get('/:path', expensiveHandler); // OK \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst4.js b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst4.js new file mode 100644 index 000000000000..ba927b872c8f --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst4.js @@ -0,0 +1,10 @@ +import express from "express"; +import rateLimit from "express-rate-limit"; + +const app = express(); + +const limiter = rateLimit(); +app.use(limiter) + +function expensiveHandler(req, res) { login(); } +app.get('/:path', expensiveHandler); // OK \ No newline at end of file