From 4b21c03fcb6b35cdfffb93ff6df26a4a2fda6bb4 Mon Sep 17 00:00:00 2001
From: goenning <me@goenning.net>
Date: Fri, 10 May 2024 06:53:25 +0100
Subject: [PATCH] better validation for redirect

---
 app/handlers/oauth.go      |  2 +-
 app/handlers/oauth_test.go | 51 +++++++++++++++++++++++++++++++++++---
 2 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/app/handlers/oauth.go b/app/handlers/oauth.go
index 3e8092191..2a8672097 100644
--- a/app/handlers/oauth.go
+++ b/app/handlers/oauth.go
@@ -242,7 +242,7 @@ func SignInByOAuth() web.HandlerFunc {
 
 		if redirect == "" {
 			redirect = c.BaseURL()
-		} else if !strings.HasPrefix(redirect, c.BaseURL()) {
+		} else if redirect != c.BaseURL() && !strings.HasPrefix(redirect, c.BaseURL()+"/") {
 			return c.Forbidden()
 		}
 
diff --git a/app/handlers/oauth_test.go b/app/handlers/oauth_test.go
index 58adcf1ca..c38b9f342 100644
--- a/app/handlers/oauth_test.go
+++ b/app/handlers/oauth_test.go
@@ -41,7 +41,37 @@ func TestSignOutHandler(t *testing.T) {
 	Expect(response.Header().Get("Set-Cookie")).ContainsSubstring("Max-Age=0; HttpOnly")
 }
 
-func TestSignInByOAuthHandler(t *testing.T) {
+func TestSignInByOAuthHandler_RootRedirect(t *testing.T) {
+	RegisterT(t)
+	bus.Init(&oauth.Service{})
+
+	server := mock.NewServer()
+	code, _ := server.
+		AddParam("provider", app.FacebookProvider).
+		AddCookie(web.CookieSessionName, "MY_SESSION_ID").
+		WithURL("http://avengers.test.fider.io/oauth/facebook?redirect=http://avengers.test.fider.io").
+		Use(middlewares.Session()).
+		Execute(handlers.SignInByOAuth())
+
+	Expect(code).Equals(http.StatusTemporaryRedirect)
+}
+
+func TestSignInByOAuthHandler_PathRedirect(t *testing.T) {
+	RegisterT(t)
+	bus.Init(&oauth.Service{})
+
+	server := mock.NewServer()
+	code, _ := server.
+		AddParam("provider", app.FacebookProvider).
+		AddCookie(web.CookieSessionName, "MY_SESSION_ID").
+		WithURL("http://avengers.test.fider.io/oauth/facebook?redirect=http://avengers.test.fider.io/something").
+		Use(middlewares.Session()).
+		Execute(handlers.SignInByOAuth())
+
+	Expect(code).Equals(http.StatusTemporaryRedirect)
+}
+
+func TestSignInByOAuthHandler_EvilRedirect(t *testing.T) {
 	RegisterT(t)
 	bus.Init(&oauth.Service{})
 
@@ -56,6 +86,21 @@ func TestSignInByOAuthHandler(t *testing.T) {
 	Expect(code).Equals(http.StatusForbidden)
 }
 
+func TestSignInByOAuthHandler_EvilRedirect2(t *testing.T) {
+	RegisterT(t)
+	bus.Init(&oauth.Service{})
+
+	server := mock.NewServer()
+	code, _ := server.
+		AddParam("provider", app.FacebookProvider).
+		AddCookie(web.CookieSessionName, "MY_SESSION_ID").
+		WithURL("http://avengers.test.fider.io/oauth/facebook?redirect=http://avengers.test.fider.io.evil.com").
+		Use(middlewares.Session()).
+		Execute(handlers.SignInByOAuth())
+
+	Expect(code).Equals(http.StatusForbidden)
+}
+
 func TestSignInByOAuthHandler_InvalidURL(t *testing.T) {
 	RegisterT(t)
 	bus.Init(&oauth.Service{})
@@ -74,7 +119,7 @@ func TestSignInByOAuthHandler_InvalidURL(t *testing.T) {
 		Execute(handlers.SignInByOAuth())
 
 	Expect(code).Equals(http.StatusTemporaryRedirect)
-	Expect(response.Header().Get("Location")).Equals("https://www.facebook.com/v3.2/dialog/oauth?client_id=FB_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%2Foauth%2Ffacebook%2Fcallback&response_type=code&scope=public_profile+email&state="+state)
+	Expect(response.Header().Get("Location")).Equals("https://www.facebook.com/v3.2/dialog/oauth?client_id=FB_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%2Foauth%2Ffacebook%2Fcallback&response_type=code&scope=public_profile+email&state=" + state)
 }
 
 func TestSignInByOAuthHandler_AuthenticatedUser(t *testing.T) {
@@ -113,7 +158,7 @@ func TestSignInByOAuthHandler_AuthenticatedUser_UsingEcho(t *testing.T) {
 	})
 
 	Expect(code).Equals(http.StatusTemporaryRedirect)
-	Expect(response.Header().Get("Location")).Equals("https://www.facebook.com/v3.2/dialog/oauth?client_id=FB_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%2Foauth%2Ffacebook%2Fcallback&response_type=code&scope=public_profile+email&state="+state)
+	Expect(response.Header().Get("Location")).Equals("https://www.facebook.com/v3.2/dialog/oauth?client_id=FB_CL_ID&redirect_uri=http%3A%2F%2Flogin.test.fider.io%2Foauth%2Ffacebook%2Fcallback&response_type=code&scope=public_profile+email&state=" + state)
 }
 
 func TestCallbackHandler_InvalidState(t *testing.T) {