From 8c8947f4a576c3ec8ec6d1af0e39fe13e07ceeff Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Fri, 20 Feb 2026 18:10:49 +0100
Subject: [PATCH] fix: consider more risky redirects

After evaluating `..`, it's possible that the resulting path is a risky
redirect, as it might be a "browser" special redirect. Detect this case.
---
 modules/httplib/url.go      | 19 ++++++++++++++++---
 modules/httplib/url_test.go |  7 +++++--
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/modules/httplib/url.go b/modules/httplib/url.go
index 32a02e3277..66ea77add7 100644
--- a/modules/httplib/url.go
+++ b/modules/httplib/url.go
@@ -1,20 +1,26 @@
 // Copyright 2023 The Gitea Authors. All rights reserved.
+// Copyright 2026 The Forgejo Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
 package httplib
 
 import (
 	"net/url"
+	"path"
 	"strings"
 
 	"forgejo.org/modules/setting"
 )
 
+// Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH"
+// Therefore we should ignore these redirect locations to prevent open redirects.
+func isBrowserRedirect(s string) bool {
+	return len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\')
+}
+
 // IsRiskyRedirectURL returns true if the URL is considered risky for redirects
 func IsRiskyRedirectURL(s string) bool {
-	// Unfortunately browsers consider a redirect Location with preceding "//", "\\", "/\" and "\/" as meaning redirect to "http(s)://REST_OF_PATH"
-	// Therefore we should ignore these redirect locations to prevent open redirects
-	if len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\') {
+	if isBrowserRedirect(s) {
 		return true
 	}
 
@@ -23,5 +29,12 @@ func IsRiskyRedirectURL(s string) bool {
 		return true
 	}
 
+	// If the path contains `..` then it's still possible this is seen
+	// as a browser redirect, use `path.Clean` to eliminate each inner `..`
+	// and then check if that might be a browser redirect.
+	if strings.Contains(u.Path, "..") {
+		return isBrowserRedirect(path.Clean(u.Path))
+	}
+
 	return false
 }
diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go
index cd2ceac267..0fd59c13a6 100644
--- a/modules/httplib/url_test.go
+++ b/modules/httplib/url_test.go
@@ -29,7 +29,6 @@ func TestIsRiskyRedirectURL(t *testing.T) {
 		{"/sub/foo", false},
 		{"http://localhost:3000/sub/foo", false},
 		{"http://localhost:3000/sub/test?param=false", false},
-		// FIXME: should probably be true (would requires resolving references using setting.appURL.ResolveReference(u))
 		{"/sub/../", false},
 		{"http://localhost:3000/sub/../", false},
 		{"/sUb/", false},
@@ -58,10 +57,12 @@ func TestIsRiskyRedirectURL(t *testing.T) {
 		{"://missing protocol scheme", true},
 		// FIXME: should probably be false
 		{"//localhost:3000/sub/test?param=false", true},
+		{"/a/../\\example.com", true},
+		{"/a/%2e%2e/\\example.com", true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.input, func(t *testing.T) {
-			assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input))
+			assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input), tt.input)
 		})
 	}
 }
@@ -114,6 +115,8 @@ func TestIsRiskyRedirectURLWithoutSubURL(t *testing.T) {
 		{"https://next.forgejo.org", true},
 		{"//next.forgejo.org/test?param=false", true},
 		{"//next.forgejo.org/sub/test?param=false", true},
+		{"/a/../\\example.com", true},
+		{"/a/%2e%2e/\\example.com", true},
 	}
 	for _, tt := range tests {
 		t.Run(tt.input, func(t *testing.T) {