Merge pull request #14600 from github/max-schaefer/express-rate-limit

JavaScript: Add support for importing `express-rate-limit` using a named import.

Author: max-schaefer

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()
+  }
 }
 
 /**

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.

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 |

javascript/ql/test/query-tests/Security/CWE-770/tst.js renamed to javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst.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

javascript/ql/test/query-tests/Security/CWE-770/tst2.ts renamed to javascript/ql/test/query-tests/Security/CWE-770/MissingRateLimit/tst2.ts

@@ -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