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

@@ -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"
+	if isBrowserRedirect(s) {
-	// Therefore we should ignore these redirect locations to prevent open redirects
-	if len(s) > 1 && (s[0] == '/' || s[0] == '\\') && (s[1] == '/' || s[1] == '\\') {
 		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
 }

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) {