GHSA-r7w9-592q-9vg4 Echo has CSRF Protection Bypass via MethodOverride Rewriting POST to Safe Method

• Go github.com/labstack/echo/v5: >= 5.0.0, < 5.4.0 → fixed in 5.4.0 • Go github.com/labstack/echo/v4: < 4.16.0 → fixed in 4.16.0

Summary The `MethodOverride` middleware accepts any non-empty string as the override target method — including safe methods (GET, HEAD, OPTIONS, TRACE) — when rewriting a POST request. When `MethodOverride` is registered before `CSRF` in the post-routing middleware chain (`e.Use()`), it rewrites the request method to a safe method before `CSRF` evaluates it. The CSRF middleware then skips token validation for the now-safe method, allowing state-changing POST handlers to execute without any CSRF check. Both the legacy token path and the modern `Sec-Fetch-Site` protection path are bypassed simultaneously. Affected Component • `middleware/method_override.go` — `MethodOverrideConfig.ToMiddleware()` (lines 55–71): no whitelist on the override target method • `middleware/csrf.go` — `CSRFConfig.ToMiddleware()` (lines 188–189): safe-method bypass branch reads the already-rewritten method • `middleware/csrf.go` — `checkSecFetchSiteRequest()` (line 283): `Sec-Fetch-Site` safe check also reads the rewritten method Description MethodOverride accepts safe methods as override targets `MethodOverrideConfig.ToMiddleware()` (`method_override.go:55-71`) restricts the source method (only POST requests are overrideable) but applies no whitelist to the target: ```go // method_override.go:61-67 req := c.Request() if req.Method == http.MethodPost { m := config.Getter(c) if m != "" { req.Method = m // ← any non-empty string accepted, including "GET" } } ``` The docstring's security claim — "For security reasons, only POST method can be overridden" — refers only to the source-method restriction. There is no guard preventing POST from being rewritten to `GET`, `HEAD`, `OPTIONS`, or `TRACE`. The test suite (`method_override_test.go`) only tests POST→DELETE and POST→PUT; POST→GET is never tested or blocked. CSRF safe-method branch reads the already-rewritten method `CSRFConfig.ToMiddleware()` (`csrf.go:188-189`) makes its validation decision based on `c.Request().Method` at the time it runs: ```go // csrf.go:188-189 switch c.Request().Method { case http.MethodGet, http.MethodHead, http.MethodOptions, http.MethodTrace: // skip validation — no token check default: // validate CSRF token ``` If MethodOverride has already run and set `req.Method = "GET"`, this `switch` falls into the skip branch regardless of the original HTTP method or what handler was matched. The `Sec-Fetch-Site`-based path in `checkSecFetchSiteRequest()` (`csrf.go:283`) is identically vulnerable: ```go // csrf.go:283 isSafe := slices.Contains(safeMethods, c.Request().Method) ``` With `req.Method = "GET"` (already rewritten), `isSafe = true` and the function returns `true` — the request is allowed even when `Sec-Fetch-Site` is `cross-site`. Both CSRF protection paths are bypassed in a single step. Execution chain In `echo.go:700`, routing and middleware wrapping are performed in one step: ```go h = applyMiddleware(e.router.Route(c), e.middleware...) ``` 1. `e.router.Route(c)` evaluates with `req.Method = "POST"` → matches and binds the POST `/transfer` handler 2. `applyMiddleware` wraps it: `MethodOverride → CSRF → postHandler` At request execution time: 3. MethodOverride runs: `req.Method == "POST"` → reads form field `_method` = `"GET"` → `req.Method = "GET"` → calls `next` 4. CSRF runs: `c.Request().Method` = `"GET"` → `case http.MethodGet` → skips token validation → calls `next` 5. postHandler runs (the handler matched in step 1 — the original POST `/transfer` state-change handler) The state-changing handler executes with no CSRF token validation and with no `Sec-Fetch-Site` check. The missing target-method whitelist is the root cause The legitimate use case for MethodOverride is allowing HTML forms (which only support GET/POST) to send PUT, DELETE, or PATCH requests. There is no legitimate scenario in which an HTML form needs to rewrite POST to GET — that can always be expressed as an ordinary GET request. Allowing POST→GET (and HEAD/OPTIONS/TRACE) serves no valid purpose and is inherently dangerous when any safe-method-aware security middleware (CSRF, logging, audit) appears downstream. 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() // MethodOverride before CSRF — both in e.Use() (post-routing) e.Use(middleware.MethodOverrideWithConfig(middleware.MethodOverrideConfig{ Getter: middleware.MethodFromForm("_method"), })) e.Use(middleware.CSRF()) e.POST("/transfer", func(c *echo.Context) error { return c.String(http.StatusOK, "transfer executed") }) e.Logger.Fatal(e.Start(":18080")) } ``` ```bash Baseline — CSRF correctly blocks a POST without a valid token curl -i -X POST 'http://127.0.0.1:18080/transfer' \ -H 'Content-Type: application/x-www-form-urlencoded' \ --data 'amount=100' HTTP/1.1 400 Bad Request (missing CSRF token) Bypass — _method=GET rewrites method before CSRF runs, skipping token validation curl -i -X POST 'http://127.0.0.1:18080/transfer' \ -H 'Content-Type: application/x-www-form-urlencoded' \ --data '_method=GET&amount=100' HTTP/1.1 200 OK transfer executed Browser-based CSRF PoC (attacker's page, auto-submits on load): document.getElementById('f').submit(); ``` The same bypass works with `MethodFromHeader` (`X-HTTP-Method-Override: GET`) and `MethodFromQuery` (`?_method=GET`). Impact • Any state-changing endpoint protected by `CSRF()` is reachable without a valid CSRF token when the vulnerable middleware ordering is present • The `Sec-Fetch-Site` modern-browser CSRF protection is bypassed simultaneously — browsers that support `Sec-Fetch-Site` provide no additional defense • Applicable attacks include: fund transfers, password/email changes, account deletion, privilege escalation, data manipulation — any POST-protected operation • Audit logs may record `GET` as the HTTP method for what were actually state-changing operations Recommended Remediation Option 1: Whitelist only unsafe methods as override targets in `MethodOverride` (preferred) The root cause is that MethodOverride accepts safe methods as targets. Restricting the override to legitimately overrideable methods (PUT, PATCH, DELETE) closes the bypass regardless of middleware ordering: ```go // method_override.go — illustrative fix var allowedOverrideMethods = map[string]bool{ http.MethodPut: true, http.MethodPatch: true, http.MethodDelete: true, } // in ToMiddleware: if req.Method == http.MethodPost { m := config.Getter(c) if m != "" && allowedOverrideMethods[m] { req.Method = m } } ``` Option 2: Use `e.Pre()` for `MethodOverride` and document the ordering requirement Placing MethodOverride in `e.Pre()` (pre-routing middleware) causes the router to match the overridden method, not the original POST. If only POST `/transfer` is registered, a POST-to-GET override results in a 405/404 — the handler never executes. Add an explicit documentation warning that MethodOverride must not be placed in `e.Use()` when CSRF middleware is also registered: ```go // MethodOverride returns a MethodOverride middleware. // // IMPORTANT: Register this middleware with e.Pre(), not e.Use(). // If registered with e.Use() before CSRF middleware, method rewriting // occurs after route matching but before CSRF validation, allowing // attackers to bypass CSRF token checks by specifying _method=GET. func MethodOverride() echo.MiddlewareFunc { ``` Option 1 is strongly preferred as it closes the vulnerability at the source and is safe regardless of middleware placement. Credit This vulnerability was discovered and reported by [bugbunny.ai](http…