GHSA-r7w9-592q-9vg4

Description

## 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): # <form method="POST" action="https://victim.example/transfer" id="f"> # <input name="_method" value="GET"> # <input name="amount" value="1000"> # <input name="to" value="attacker_account"> # </form> # <script>document.getElementById('f').submit();</script> ``` 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](https://bugbunny.ai). ## Patches Fixed in `github.com/labstack/echo/v5` v5.4.0 and `github.com/labstack/echo/v4` v4.16.0. The MethodOverride middleware no longer overrides a POST to `GET`, `HEAD`, `OPTIONS`, `TRACE` or `CONNECT`. ## Workarounds Register MethodOverride with `Echo#Pre` (as documented), register CSRF before MethodOverride, or use the default header getter (`X-HTTP-Method-Override`), which a cross-site form cannot set.