fix

Author: wxiaoguang

modules/httplib/url.go

@@ -8,20 +8,32 @@ import (
 	"strings"
 
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/util"
 )
 
-// IsRiskyRedirectURL returns true if the URL is considered risky for redirects
-func IsRiskyRedirectURL(s string) bool {
+func isRelativeURL(u *url.URL) bool {
+	return u.Scheme == "" && u.Host == ""
+}
+
+// IsRelativeURL detects if a URL is relative (no scheme or host)
+func IsRelativeURL(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] == '\\') {
-		return true
+		return false
 	}
-
 	u, err := url.Parse(s)
-	if err != nil || ((u.Scheme != "" || u.Host != "") && !strings.HasPrefix(strings.ToLower(s), strings.ToLower(setting.AppURL))) {
+	return err == nil && isRelativeURL(u)
+}
+
+func IsCurrentGiteaSiteURL(s string) bool {
+	if IsRelativeURL(s) {
 		return true
 	}
-
-	return false
+	u, err := url.Parse(s)
+	if err != nil {
+		return false
+	}
+	u.Path = util.PathJoinRelX(u.Path)
+	return strings.HasPrefix(strings.ToLower(u.String()+"/"), strings.ToLower(setting.AppURL))
 }

modules/httplib/url_test.go

@@ -7,32 +7,40 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/test"
 
 	"github.com/stretchr/testify/assert"
 )
 
-func TestIsRiskyRedirectURL(t *testing.T) {
-	setting.AppURL = "http://localhost:3000/"
-	tests := []struct {
-		input string
-		want  bool
-	}{
-		{"", false},
-		{"foo", false},
-		{"/", false},
-		{"/foo?k=%20#abc", false},
-
-		{"//", true},
-		{"\\\\", true},
-		{"/\\", true},
-		{"\\/", true},
-		{"mail:[email protected]", true},
-		{"https://test.com", true},
-		{setting.AppURL + "/foo", false},
+func TestIsRelativeURL(t *testing.T) {
+	rel := []string{
+		"",
+		"foo",
+		"/",
+		"/foo?k=%20#abc",
+	}
+	for _, s := range rel {
+		assert.True(t, IsRelativeURL(s), "rel = %q", s)
+	}
+	abs := []string{
+		"//",
+		"\\\\",
+		"/\\",
+		"\\/",
+		"mailto:[email protected]",
+		"https://test.com",
 	}
-	for _, tt := range tests {
-		t.Run(tt.input, func(t *testing.T) {
-			assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input))
-		})
+	for _, s := range abs {
+		assert.False(t, IsRelativeURL(s), "abs = %q", s)
 	}
 }
+
+func TestIsCurrentGiteaSiteURL(t *testing.T) {
+	defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")()
+	assert.True(t, IsCurrentGiteaSiteURL("/foo"))
+	assert.True(t, IsCurrentGiteaSiteURL("http://localhost:3000/sub"))
+	assert.True(t, IsCurrentGiteaSiteURL("http://localhost:3000/sub/"))
+	assert.False(t, IsCurrentGiteaSiteURL("http://localhost:3000/sub/.."))
+	assert.False(t, IsCurrentGiteaSiteURL("http://localhost:3000/other"))
+	assert.False(t, IsCurrentGiteaSiteURL("http://other/"))
+}

routers/common/redirect.go

@@ -17,7 +17,7 @@ func FetchRedirectDelegate(resp http.ResponseWriter, req *http.Request) {
 	// The typical page is "issue comment" page. The backend responds "/owner/repo/issues/1#comment-2",
 	// then frontend needs this delegate to redirect to the new location with hash correctly.
 	redirect := req.PostFormValue("redirect")
-	if httplib.IsRiskyRedirectURL(redirect) {
+	if !httplib.IsCurrentGiteaSiteURL(redirect) {
 		resp.WriteHeader(http.StatusBadRequest)
 		return
 	}

services/context/context_response.go

@@ -51,7 +51,7 @@ func (ctx *Context) RedirectToFirst(location ...string) {
 			continue
 		}
 
-		if httplib.IsRiskyRedirectURL(loc) {
+		if !httplib.IsCurrentGiteaSiteURL(loc) {
 			continue
 		}