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..61d75a519 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,38 @@ component extends="wheels.WheelsTest" { expect(result3).toInclude("Rate limit exceeded"); }); + it("defaults to last proxy strategy when trustProxy is enabled without explicit proxyStrategy", function() { + var limiter = new wheels.middleware.RateLimiter( + trustProxy = true, + maxRequests = 1, + windowSeconds = 60 + ); + + 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.50", + http_x_forwarded_for: "1.1.1.1, 10.0.0.1" + } + }; + 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() { expect(function() { new wheels.middleware.RateLimiter(proxyStrategy = "middle");