From 1b29c32f7d9c90b6474cbb3f847229660d776b6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mat=C3=ADas=20Insaurralde?= <matias@insaurral.de>
Date: Tue, 7 Apr 2026 21:03:58 -0300
Subject: [PATCH] appview/state: fix open redirect via return_url after OAuth
 login
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Validate return_url before storing it in the session: only relative
paths starting with "/" (and not "//") are accepted. Anything else —
absolute URLs and protocol-relative URLs — is replaced with "/".

Add tests covering the accepted and rejected cases.

Signed-off-by: Matías Insaurralde <matias@insaurral.de>
---
 appview/state/login.go      | 12 +++++++++++-
 appview/state/login_test.go | 28 ++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 appview/state/login_test.go

diff --git a/appview/state/login.go b/appview/state/login.go
index c9174252..e923f091 100644
--- a/appview/state/login.go
+++ b/appview/state/login.go
@@ -106,7 +106,7 @@ func (s *State) Login(w http.ResponseWriter, r *http.Request) {
 			}
 		}
 
-		if err := s.oauth.SetAuthReturn(w, r, returnURL, addAccount); err != nil {
+		if err := s.oauth.SetAuthReturn(w, r, sanitizeReturnURL(returnURL), addAccount); err != nil {
 			l.Error("failed to set auth return", "err", err)
 		}
 
@@ -125,6 +125,16 @@ func (s *State) Login(w http.ResponseWriter, r *http.Request) {
 	}
 }
 
+// sanitizeReturnURL ensures the return URL is a relative path on the same
+// origin. Anything else — absolute URLs, protocol-relative URLs — is replaced
+// with "/" to prevent open redirect after OAuth login.
+func sanitizeReturnURL(s string) string {
+	if strings.HasPrefix(s, "/") && !strings.HasPrefix(s, "//") {
+		return s
+	}
+	return "/"
+}
+
 func (s *State) Logout(w http.ResponseWriter, r *http.Request) {
 	l := s.logger.With("handler", "Logout")
 
diff --git a/appview/state/login_test.go b/appview/state/login_test.go
new file mode 100644
index 00000000..dd72c31c
--- /dev/null
+++ b/appview/state/login_test.go
@@ -0,0 +1,28 @@
+package state
+
+import "testing"
+
+func TestSanitizeReturnURL(t *testing.T) {
+	cases := []struct {
+		input string
+		want  string
+	}{
+		{"/", "/"},
+		{"/some/path", "/some/path"},
+		{"/valid?query=1", "/valid?query=1"},
+		{"/valid#anchor", "/valid#anchor"},
+		// External URLs must be rejected.
+		{"https://evil.com", "/"},
+		{"http://evil.com", "/"},
+		// Protocol-relative URLs are treated as external by browsers.
+		{"//evil.com", "/"},
+		{"//evil.com/phishing", "/"},
+		// Empty string.
+		{"", "/"},
+	}
+	for _, tc := range cases {
+		if got := sanitizeReturnURL(tc.input); got != tc.want {
+			t.Errorf("sanitizeReturnURL(%q) = %q, want %q", tc.input, got, tc.want)
+		}
+	}
+}
-- 
2.51.2