From 3970ead7abb8a4b4366bc220bad8ce7cdab2f602 Mon Sep 17 00:00:00 2001
From: Max Schaefer <max@semmle.com>
Date: Wed, 18 Sep 2019 12:13:23 +0100
Subject: [PATCH] JavaScript: Add support for `rate-limiter-flexible` package.

---
 change-notes/1.23/analysis-javascript.md      |  1 +
 .../security/dataflow/MissingRateLimiting.qll | 41 +++++++++++++++++++
 .../test/query-tests/Security/CWE-770/tst.js  |  8 ++++
 3 files changed, 50 insertions(+)

diff --git a/change-notes/1.23/analysis-javascript.md b/change-notes/1.23/analysis-javascript.md
index 5f9e1f37209..fea5094ecfd 100644
--- a/change-notes/1.23/analysis-javascript.md
+++ b/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.
 
diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/MissingRateLimiting.qll b/javascript/ql/src/semmle/javascript/security/dataflow/MissingRateLimiting.qll
index 1bfb0b51a9a..0942cbb1af9 100644
--- a/javascript/ql/src/semmle/javascript/security/dataflow/MissingRateLimiting.qll
+++ b/javascript/ql/src/semmle/javascript/security/dataflow/MissingRateLimiting.qll
@@ -155,3 +155,44 @@ 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 functions that invoke the `consume` method of one of the `RateLimiter*`
+ * classes from the `rate-limiter-flexible` package on a property of their first argument,
+ * 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
+    |
+      rateLimiterClassName.matches("RateLimiter%") and
+      rateLimiterClass = DataFlow::moduleMember("rate-limiter-flexible", rateLimiterClassName) and
+      rateLimiterInstance = rateLimiterClass.getAnInstantiation() and
+      getParameter(0).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)
+  }
+}
diff --git a/javascript/ql/test/query-tests/Security/CWE-770/tst.js b/javascript/ql/test/query-tests/Security/CWE-770/tst.js
index f38a9e83aa2..3d00cf59844 100644
--- a/javascript/ql/test/query-tests/Security/CWE-770/tst.js
+++ b/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);