From fb1df1c49d66ebb51f6d49a32d0d607636545441 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Fri, 10 Apr 2026 02:54:41 -0700 Subject: [PATCH 1/2] fix(middleware): default rate limiter proxy strategy to last for security Change proxyStrategy default from "first" (spoofable leftmost IP) to "last" (rightmost proxy-appended IP) when trustProxy is enabled. This prevents attackers from bypassing rate limiting via X-Forwarded-For header spoofing. Co-Authored-By: Claude Opus 4.6 (1M context) --- vendor/wheels/middleware/RateLimiter.cfc | 10 +++++----- .../specs/middleware/RateLimiterSpec.cfc | 20 ++++++++++++++++++- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/vendor/wheels/middleware/RateLimiter.cfc b/vendor/wheels/middleware/RateLimiter.cfc index 0e87192f8..c269f2679 100644 --- a/vendor/wheels/middleware/RateLimiter.cfc +++ b/vendor/wheels/middleware/RateLimiter.cfc @@ -23,10 +23,10 @@ component implements="wheels.middleware.MiddlewareInterface" output="false" { * Without a proxy that sanitizes this header, any client can spoof arbitrary IPs to bypass rate * limiting entirely. Your proxy MUST be configured to either: (a) drop incoming X-Forwarded-For and * set it to the real client IP, or (b) append the client IP so the rightmost entry is trustworthy. - * If your proxy appends, set proxyStrategy="last" to use the rightmost (proxy-added) IP. - * @proxyStrategy Which IP to extract from X-Forwarded-For: "first" (leftmost, client-supplied — default - * for backwards compatibility) or "last" (rightmost, added by the nearest trusted proxy — recommended - * when the proxy appends rather than overwrites the header). + * If your proxy appends, the default proxyStrategy="last" uses the rightmost (proxy-added) IP. + * @proxyStrategy Which IP to extract from X-Forwarded-For: "last" (rightmost, added by the nearest + * trusted proxy — default, secure when the proxy appends the real client IP) or "first" (leftmost, + * client-supplied — available for backward compatibility but vulnerable to spoofing). * @maxStoreSize Maximum number of entries allowed in the in-memory store. When exceeded during cleanup, * the oldest entries are evicted. Prevents unbounded memory growth from attackers rotating client keys. * Only applies when storage="memory". Default: 100000. @@ -46,7 +46,7 @@ component implements="wheels.middleware.MiddlewareInterface" output="false" { any keyFunction = "", string headerPrefix = "X-RateLimit", boolean trustProxy = false, - string proxyStrategy = "first", + string proxyStrategy = "last", numeric maxStoreSize = 100000, numeric maxTimestampsPerKey = 0, boolean failOpen = false diff --git a/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc b/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc index 7f0cb7ce8..037814de5 100644 --- a/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc +++ b/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc @@ -170,7 +170,7 @@ component extends="wheels.WheelsTest" { describe("RateLimiter proxyStrategy", function() { - it("defaults to first IP in X-Forwarded-For chain", function() { + it("uses first IP in X-Forwarded-For chain when proxyStrategy is first", function() { var limiter = new wheels.middleware.RateLimiter( maxRequests = 1, windowSeconds = 60, @@ -275,6 +275,24 @@ component extends="wheels.WheelsTest" { expect(result3).toInclude("Rate limit exceeded"); }); + it("defaults to last proxy strategy when trustProxy is enabled", function() { + var limiter = new wheels.middleware.RateLimiter( + trustProxy = true, + maxRequests = 5, + windowSeconds = 60 + ); + + // The rightmost IP should be used (proxy-appended) + var req = { + cgi: { + remote_addr: "10.0.0.1", + http_x_forwarded_for: "1.2.3.4, 5.6.7.8" + } + }; + var clientKey = limiter.$getClientKey(req); + expect(clientKey).toBe("5.6.7.8"); + }); + it("throws on invalid proxyStrategy", function() { expect(function() { new wheels.middleware.RateLimiter(proxyStrategy = "middle"); From 2dea5328071acd640f258e0cfbab4f5e547c8504 Mon Sep 17 00:00:00 2001 From: Peter Amiri Date: Fri, 10 Apr 2026 04:42:18 -0700 Subject: [PATCH 2/2] fix(middleware): fix test to use public handle() api instead of private method The test was calling $getClientKey which doesn't exist. Rewrite to use handle() and verify rate limiting behavior proves the default strategy is "last" (same last IP = same bucket = blocked on second request). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../specs/middleware/RateLimiterSpec.cfc | 30 ++++++++++++++----- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc b/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc index 037814de5..61d75a519 100644 --- a/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc +++ b/vendor/wheels/tests/specs/middleware/RateLimiterSpec.cfc @@ -275,22 +275,36 @@ component extends="wheels.WheelsTest" { expect(result3).toInclude("Rate limit exceeded"); }); - it("defaults to last proxy strategy when trustProxy is enabled", function() { + it("defaults to last proxy strategy when trustProxy is enabled without explicit proxyStrategy", function() { var limiter = new wheels.middleware.RateLimiter( trustProxy = true, - maxRequests = 5, + maxRequests = 1, windowSeconds = 60 ); - // The rightmost IP should be used (proxy-appended) - var req = { + var nextFn = function(req) { return "ok"; }; + + // Two requests with different first IPs but same last IP should share a bucket + // (proving the default strategy is "last", not "first") + var req1 = { cgi: { - remote_addr: "10.0.0.1", - http_x_forwarded_for: "1.2.3.4, 5.6.7.8" + remote_addr: "10.0.0.50", + http_x_forwarded_for: "1.1.1.1, 10.0.0.1" } }; - var clientKey = limiter.$getClientKey(req); - expect(clientKey).toBe("5.6.7.8"); + var req2 = { + cgi: { + remote_addr: "10.0.0.50", + http_x_forwarded_for: "2.2.2.2, 10.0.0.1" + } + }; + + var result1 = limiter.handle(request = req1, next = nextFn); + expect(result1).toBe("ok"); + + // Second request from "different" first IP but SAME last IP should be blocked + var result2 = limiter.handle(request = req2, next = nextFn); + expect(result2).toInclude("Rate limit exceeded"); }); it("throws on invalid proxyStrategy", function() {