Threats tagged 'ghsa-246p-cpwv-v3jq'

## 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()