Merge pull request github#1950 from xiemaisi/js/rate-limiter-flexible

Approved by esben-semmle

Author: semmle-qlci

change-notes/1.23/analysis-javascript.md

@@ -6,6 +6,7 @@
   - [firebase](https://www.npmjs.com/package/firebase)
   - [mongodb](https://www.npmjs.com/package/mongodb)
   - [mongoose](https://www.npmjs.com/package/mongoose)
+  - [rate-limiter-flexible](https://www.npmjs.com/package/rate-limiter-flexible)
 
 * The call graph has been improved to resolve method calls in more cases. This may produce more security alerts.
 

javascript/ql/src/semmle/javascript/security/dataflow/MissingRateLimiting.qll

@@ -23,6 +23,7 @@
  */
 
 import javascript
+private import semmle.javascript.frameworks.ConnectExpressShared::ConnectExpressShared
 
 // main concepts
 /**
@@ -155,3 +156,43 @@ class RouteHandlerLimitedByExpressLimiter extends RateLimitedRouteHandlerExpr {
     )
   }
 }
+
+/**
+ * A rate-handler function implemented using one of the rate-limiting classes provided
+ * by the `rate-limiter-flexible` package.
+ *
+ * We look for route handlers that invoke the `consume` method of one of the `RateLimiter*`
+ * classes from the `rate-limiter-flexible` package on a property of their request parameter,
+ * like the `rateLimiterMiddleware` function in this example:
+ *
+ * ```
+ * import { RateLimiterRedis } from 'rate-limiter-flexible';
+ * const rateLimiter = new RateLimiterRedis(...);
+ * function rateLimiterMiddleware(req, res, next) {
+ *   rateLimiter.consume(req.ip).then(next).catch(res.status(429).send('rate limited'));
+ * }
+ * ```
+ */
+class RateLimiterFlexibleRateLimiter extends DataFlow::FunctionNode {
+  RateLimiterFlexibleRateLimiter() {
+    exists(
+      string rateLimiterClassName, DataFlow::SourceNode rateLimiterClass,
+      DataFlow::SourceNode rateLimiterInstance, DataFlow::ParameterNode request
+    |
+      rateLimiterClassName.matches("RateLimiter%") and
+      rateLimiterClass = DataFlow::moduleMember("rate-limiter-flexible", rateLimiterClassName) and
+      rateLimiterInstance = rateLimiterClass.getAnInstantiation() and
+      request.getParameter() = getRouteHandlerParameter(getFunction(), "request") and
+      request.getAPropertyRead() = rateLimiterInstance.getAMemberCall("consume").getAnArgument()
+    )
+  }
+}
+
+/**
+ * A route-handler expression that is rate-limited by the `rate-limiter-flexible` package.
+ */
+class RouteHandlerLimitedByRateLimiterFlexible extends RateLimiter {
+  RouteHandlerLimitedByRateLimiterFlexible() {
+    any(RateLimiterFlexibleRateLimiter rl).flowsToExpr(this)
+  }
+}

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

@@ -63,3 +63,11 @@ app3.get('/:path', expensiveHandler1); // OK
 
 express().get('/:path', function(req, res) { verifyUser(req); });  // NOT OK
 express().get('/:path', RateLimit(), function(req, res) { verifyUser(req); });  // OK
+
+// rate limiting using rate-limiter-flexible
+const { RateLimiterRedis } = require('rate-limiter-flexible');
+const rateLimiter = new RateLimiterRedis();
+const rateLimiterMiddleware = (req, res, next) => {
+  rateLimiter.consume(req.ip).then(next).catch(res.status(429).send('rate limited'));
+};
+express().get('/:path', rateLimiterMiddleware, expensiveHandler1);