From 59f8e943fb8c14ad9a39cccdd7cf4c0c432d07ec Mon Sep 17 00:00:00 2001
From: Bryan Frimin <bryan@probo.com>
Date: Tue, 26 May 2026 11:41:33 -0700
Subject: [PATCH] Fix open redirect bypass in saferedirect

Relative redirect URLs are now normalized before validation. Paths
containing backslashes (including percent-encoded %5c) are rejected,
closing a bypass where /../\evil.com passed checks but http.Redirect
normalized to /\evil.com.

Reported by Fushuling and RacerZ.

Signed-off-by: Bryan Frimin <bryan@probo.com>
---
 SECURITY_NOTES.md                     | 17 +++++++++++
 pkg/saferedirect/saferedirect.go      | 31 +++++++++++++++++--
 pkg/saferedirect/saferedirect_test.go | 43 +++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 2 deletions(-)

diff --git a/SECURITY_NOTES.md b/SECURITY_NOTES.md
index 66ac8b46c..d4f6f7d33 100644
--- a/SECURITY_NOTES.md
+++ b/SECURITY_NOTES.md
@@ -3,6 +3,23 @@
 User-facing notes on security-relevant changes to Probo. For the
 vulnerability reporting process, see [SECURITY.md](SECURITY.md).
 
+## Open redirect bypass in saferedirect
+
+_2026-05-26, Auth_
+
+Relative redirect URLs are now normalized before validation. Paths
+containing backslashes (including percent-encoded `%5c`) are rejected,
+and the cleaned path is checked for protocol-relative and backslash
+prefixes.
+
+Previously, `Validate` only inspected the second character of the raw
+input. A path like `/../\evil.com` passed validation because the second
+character is `.`, but Go's `http.Redirect` normalized it to `/\evil.com`,
+which browsers can treat as an external redirect.
+
+Reported by [Fushuling](https://github.com/Fushuling) and
+[RacerZ](https://github.com/RacerZ-fighting).
+
 ## Password changes invalidate existing sessions
 
 _2026-04-29, IAM_
diff --git a/pkg/saferedirect/saferedirect.go b/pkg/saferedirect/saferedirect.go
index 8e855c7e3..6ed11932f 100644
--- a/pkg/saferedirect/saferedirect.go
+++ b/pkg/saferedirect/saferedirect.go
@@ -18,6 +18,7 @@ import (
 	"context"
 	"net/http"
 	"net/url"
+	"path"
 	"strings"
 )
 
@@ -54,11 +55,12 @@ func (sr *SafeRedirect) Validate(ctx context.Context, redirectURL string) (strin
 	}
 
 	if strings.HasPrefix(redirectURL, "/") {
-		if len(redirectURL) > 1 && (redirectURL[1] == '/' || redirectURL[1] == '\\') {
+		safePath, ok := normalizeRelativePath(redirectURL)
+		if !ok {
 			return "", false
 		}
 
-		return redirectURL, true
+		return safePath, true
 	}
 
 	parsedURL, err := url.Parse(redirectURL)
@@ -93,3 +95,28 @@ func (sr *SafeRedirect) Redirect(w http.ResponseWriter, r *http.Request, redirec
 	safeURL := sr.GetSafeRedirectURL(r.Context(), redirectURL, fallbackURL)
 	http.Redirect(w, r, safeURL, statusCode)
 }
+
+func normalizeRelativePath(redirectURL string) (string, bool) {
+	if strings.HasPrefix(redirectURL, "//") {
+		return "", false
+	}
+
+	if strings.Contains(redirectURL, `\`) || strings.Contains(strings.ToLower(redirectURL), "%5c") {
+		return "", false
+	}
+
+	cleaned := path.Clean(redirectURL)
+	if !strings.HasPrefix(cleaned, "/") {
+		return "", false
+	}
+
+	if len(cleaned) > 1 && (cleaned[1] == '/' || cleaned[1] == '\\') {
+		return "", false
+	}
+
+	if strings.Contains(cleaned, `\`) {
+		return "", false
+	}
+
+	return cleaned, true
+}
diff --git a/pkg/saferedirect/saferedirect_test.go b/pkg/saferedirect/saferedirect_test.go
index 5c68cdb00..696c8b2d0 100644
--- a/pkg/saferedirect/saferedirect_test.go
+++ b/pkg/saferedirect/saferedirect_test.go
@@ -103,6 +103,34 @@ func TestSafeRedirect_Validate(t *testing.T) {
 			expectedURL:     "",
 			expectedIsValid: false,
 		},
+		{
+			name:            "path traversal backslash bypass",
+			allowedHost:     saferedirect.StaticHosts("example.com"),
+			redirectURL:     "/../\\evil.com/phishing",
+			expectedURL:     "",
+			expectedIsValid: false,
+		},
+		{
+			name:            "embedded backslash",
+			allowedHost:     saferedirect.StaticHosts("example.com"),
+			redirectURL:     "/foo/..\\evil.com/phishing",
+			expectedURL:     "",
+			expectedIsValid: false,
+		},
+		{
+			name:            "percent-encoded backslash",
+			allowedHost:     saferedirect.StaticHosts("example.com"),
+			redirectURL:     "/%5cevil.com/phishing",
+			expectedURL:     "",
+			expectedIsValid: false,
+		},
+		{
+			name:            "path normalization",
+			allowedHost:     saferedirect.StaticHosts("example.com"),
+			redirectURL:     "/foo/../dashboard",
+			expectedURL:     "/dashboard",
+			expectedIsValid: true,
+		},
 	}
 
 	for _, tt := range tests {
@@ -168,6 +196,13 @@ func TestSafeRedirect_GetSafeRedirectURL(t *testing.T) {
 			fallbackURL: "/home",
 			expectedURL: "/home",
 		},
+		{
+			name:        "path traversal backslash bypass",
+			allowedHost: saferedirect.StaticHosts("example.com"),
+			redirectURL: "/../\\evil.com/phishing",
+			fallbackURL: "/home",
+			expectedURL: "/home",
+		},
 	}
 
 	for _, tt := range tests {
@@ -235,6 +270,14 @@ func TestSafeRedirect_Redirect(t *testing.T) {
 			expectedStatus: http.StatusFound,
 			expectedURL:    "/home",
 		},
+		{
+			name:           "path traversal backslash bypass",
+			allowedHost:    saferedirect.StaticHosts("example.com"),
+			redirectURL:    "/../\\evil.com/phishing",
+			fallbackURL:    "/home",
+			expectedStatus: http.StatusFound,
+			expectedURL:    "/home",
+		},
 	}
 
 	for _, tt := range tests {