Validate returnTo at creation (scheme injection / open redirect) #57

New issue

Open

opened 2026-09-19 01:23:15 +00:00 by ginjiruu · 0 comments

ginjiruu commented

2026-09-19 01:23:15 +00:00

Owner

Status (updated 21 Sep 2026)

Partially addressed by 2c7699b. A validReturnTo check now runs at the callback (before the exchange): an absolute returnTo whose host is not one of the App's registered hostnames, or that carries userinfo, is rejected with 403. This closes the cross-host open-redirect vector.

Still open — the scheme-injection vector is not closed. validReturnTo treats any hostless URL as safe (u.Host == "" → true), so a non-https scheme such as javascript:/data: (planted via X-Forwarded-Proto) still passes. The issue's core requirement — https-only — is unmet, as is applying the check at creation (relogin) with a fallback to the App root (it is applied at the callback, failing closed with 403 instead).

Remaining work:

Enforce https-only (reject non-https schemes, including hostless javascript:/data:).
Move the check to the seam where returnTo is created (relogin) so the State cookie can never carry an unsafe value; fall back to the App root on an invalid value.
Add the hostile-corpus tests (javascript:, data:, cross-host, userinfo, missing scheme).

Context

Security architecture review, 18 Sep 2026 (candidate 3, "Worth exploring"). The returnTo value is built from the ext_authz attributes (scheme://host + path + query) at relogin, stored in the signed State cookie, and becomes the callback's post-login Location. The host part is safe — it is the App's own hostname, the same value that resolved the App. But the scheme comes from X-Forwarded-Proto via the gateway: an attacker who can influence XFP can plant a non-https scheme (e.g. javascript:) into the signed cookie and have the browser redirected there after the victim's login. Current browser hardening (stripping javascript:/data: from Location) limits practical impact today, but the invariant "the post-login redirect target is an https URL on the App's host" is enforced nowhere. Notably, the logout flow already enforces the analogous invariant for its Referer-based redirect — the callback path doesn't apply the rule the codebase already knows.

What to build

A small pure validator module for the return-To value (https only; host equals the App's hostname; no userinfo), applied once at the seam where returnTo is created (relogin), so the State cookie can never carry an unsafe value. An invalid returnTo falls back to a safe default (the App's root). The callback's redirect then becomes a trust-the-cookie path, and the open-redirect rule lives in exactly one place both flows share.

Acceptance criteria

A pure return-To validator exists (https only, host == the App's hostname, no userinfo) with a test corpus of hostile schemes/hosts (javascript:, data:, cross-host, userinfo, missing scheme) — partial: host + no-userinfo done in 2c7699b; https-only + corpus outstanding
The validator is applied at relogin where returnTo is created; the State cookie can never carry an unsafe value — currently applied at the callback (403), not at creation
An invalid returnTo falls back to a safe default (the App's root) — currently 403, not a fallback
Existing relogin/callback tests stay green; new tests cover the fallback and the hostile corpus — callback host-rejection tests added in 2c7699b; fallback + scheme corpus outstanding

Blocked by

None — can start immediately

## Status (updated 21 Sep 2026) **Partially addressed by `2c7699b`.** A `validReturnTo` check now runs at the callback (before the exchange): an absolute returnTo whose host is not one of the App's registered hostnames, or that carries userinfo, is rejected with 403. This closes the **cross-host open-redirect** vector. **Still open — the scheme-injection vector is not closed.** `validReturnTo` treats any hostless URL as safe (`u.Host == ""` → `true`), so a non-https scheme such as `javascript:`/`data:` (planted via `X-Forwarded-Proto`) still passes. The issue's core requirement — **https-only** — is unmet, as is applying the check **at creation (relogin)** with a fallback to the App root (it is applied at the callback, failing closed with 403 instead). Remaining work: - Enforce https-only (reject non-https schemes, including hostless `javascript:`/`data:`). - Move the check to the seam where returnTo is created (relogin) so the State cookie can never carry an unsafe value; fall back to the App root on an invalid value. - Add the hostile-corpus tests (javascript:, data:, cross-host, userinfo, missing scheme). ## Context Security architecture review, 18 Sep 2026 (candidate 3, "Worth exploring"). The returnTo value is built from the ext_authz attributes (`scheme://host + path + query`) at relogin, stored in the signed State cookie, and becomes the callback's post-login `Location`. The host part is safe — it is the App's own hostname, the same value that resolved the App. But the scheme comes from `X-Forwarded-Proto` via the gateway: an attacker who can influence XFP can plant a non-https scheme (e.g. `javascript:`) into the signed cookie and have the browser redirected there after the victim's login. Current browser hardening (stripping `javascript:`/`data:` from `Location`) limits practical impact today, but the invariant "the post-login redirect target is an https URL on the App's host" is enforced nowhere. Notably, the logout flow already enforces the analogous invariant for its Referer-based redirect — the callback path doesn't apply the rule the codebase already knows. ## What to build A small pure validator module for the return-To value (https only; host equals the App's hostname; no userinfo), applied once at the seam where returnTo is created (relogin), so the State cookie can never carry an unsafe value. An invalid returnTo falls back to a safe default (the App's root). The callback's redirect then becomes a trust-the-cookie path, and the open-redirect rule lives in exactly one place both flows share. ## Acceptance criteria - [ ] A pure return-To validator exists (https only, host == the App's hostname, no userinfo) with a test corpus of hostile schemes/hosts (javascript:, data:, cross-host, userinfo, missing scheme) — *partial: host + no-userinfo done in `2c7699b`; https-only + corpus outstanding* - [ ] The validator is applied at relogin where returnTo is created; the State cookie can never carry an unsafe value — *currently applied at the callback (403), not at creation* - [ ] An invalid returnTo falls back to a safe default (the App's root) — *currently 403, not a fallback* - [ ] Existing relogin/callback tests stay green; new tests cover the fallback and the hostile corpus — *callback host-rejection tests added in `2c7699b`; fallback + scheme corpus outstanding* ## Blocked by - None — can start immediately

ginjiruu added the

ready-for-agent

label

2026-09-19 01:25:26 +00:00