fix(security): block open redirect in login redirect target
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>
This commit is contained in:
+15
-3
@@ -543,11 +543,23 @@ func decodeState(state string) string {
|
||||
if err != nil {
|
||||
return "/"
|
||||
}
|
||||
redirect := string(data)
|
||||
if !strings.HasPrefix(redirect, "/") {
|
||||
return safeRedirectPath(string(data))
|
||||
}
|
||||
|
||||
// safeRedirectPath 仅放行站内相对路径,拦截开放重定向。浏览器会忽略 URL 中的
|
||||
// Tab/换行/回车,并把 //host 或 /\host 解析为协议相对的跨站地址,因此先剥离这些
|
||||
// 控制字符,再拒绝 // 与 /\ 前缀。
|
||||
func safeRedirectPath(redirect string) string {
|
||||
cleaned := strings.Map(func(r rune) rune {
|
||||
if r == '\t' || r == '\n' || r == '\r' {
|
||||
return -1
|
||||
}
|
||||
return r
|
||||
}, redirect)
|
||||
if !strings.HasPrefix(cleaned, "/") || strings.HasPrefix(cleaned, "//") || strings.HasPrefix(cleaned, "/\\") {
|
||||
return "/"
|
||||
}
|
||||
return redirect
|
||||
return cleaned
|
||||
}
|
||||
|
||||
func RequestOrigin(r *http.Request) string {
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user