5ca5b72b01
The login redirect accepted any value beginning with a single slash, so a protocol-relative URL such as "//evil.com" (or a backslash variant) slipped through and the browser resolved it to an external site. Both the Go OAuth state decoder and the web login page used the same prefix-only check, so an attacker could send a victim to /login?redirect=//evil.com — or supply it via the Linux.do OAuth redirect param — and bounce them off-site after login. Harden both layers: strip Tab/CR/LF (which browsers ignore inside URLs) and reject protocol-relative and backslash-prefixed targets, allowing only genuine same-site relative paths. Detected by Aeon + semgrep (go.lang.security.injection.open-redirect). Severity: medium CWE-601 (URL Redirection to Untrusted Site) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
42 lines
1.2 KiB
Go
42 lines
1.2 KiB
Go
package service
|
|
|
|
import (
|
|
"encoding/base64"
|
|
"testing"
|
|
)
|
|
|
|
func TestSafeRedirectPath(t *testing.T) {
|
|
cases := map[string]string{
|
|
"/": "/",
|
|
"/canvas/abc": "/canvas/abc",
|
|
"/login?redirect=/x": "/login?redirect=/x",
|
|
"": "/",
|
|
"//evil.com": "/",
|
|
"/\\evil.com": "/",
|
|
"https://evil.com": "/",
|
|
"http://evil.com": "/",
|
|
"javascript:alert(1)": "/",
|
|
"evil.com": "/",
|
|
"/\t/evil.com": "/", // browsers strip the tab → //evil.com
|
|
"/normal\tpath": "/normalpath",
|
|
}
|
|
for in, want := range cases {
|
|
if got := safeRedirectPath(in); got != want {
|
|
t.Errorf("safeRedirectPath(%q) = %q, want %q", in, got, want)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestDecodeStateRejectsOpenRedirect(t *testing.T) {
|
|
for _, in := range []string{"//evil.com", "/\\evil.com", "https://evil.com"} {
|
|
state := base64.RawURLEncoding.EncodeToString([]byte(in))
|
|
if got := decodeState(state); got != "/" {
|
|
t.Errorf("decodeState(state(%q)) = %q, want \"/\"", in, got)
|
|
}
|
|
}
|
|
state := base64.RawURLEncoding.EncodeToString([]byte("/canvas/1"))
|
|
if got := decodeState(state); got != "/canvas/1" {
|
|
t.Errorf("decodeState(state(/canvas/1)) = %q, want /canvas/1", got)
|
|
}
|
|
}
|