summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--modules/httplib/url.go34
-rw-r--r--modules/httplib/url_test.go77
-rw-r--r--routers/common/redirect.go2
-rw-r--r--routers/web/auth/auth.go6
-rw-r--r--routers/web/auth/oauth.go2
-rw-r--r--routers/web/auth/password.go2
-rw-r--r--routers/web/repo/repo.go2
-rw-r--r--routers/web/web.go2
-rw-r--r--services/context/context_response.go6
9 files changed, 93 insertions, 40 deletions
diff --git a/modules/httplib/url.go b/modules/httplib/url.go
index 14b95898f5..b679b44500 100644
--- a/modules/httplib/url.go
+++ b/modules/httplib/url.go
@@ -8,20 +8,40 @@ 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 urlIsRelative(s string, u *url.URL) 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
}
+ return u != nil && u.Scheme == "" && u.Host == ""
+}
+// IsRelativeURL detects if a URL is relative (no scheme or host)
+func IsRelativeURL(s string) bool {
u, err := url.Parse(s)
- if err != nil || ((u.Scheme != "" || u.Host != "") && !strings.HasPrefix(strings.ToLower(s), strings.ToLower(setting.AppURL))) {
- return true
- }
+ return err == nil && urlIsRelative(s, u)
+}
- return false
+func IsCurrentGiteaSiteURL(s string) bool {
+ u, err := url.Parse(s)
+ if err != nil {
+ return false
+ }
+ if u.Path != "" {
+ u.Path = "/" + util.PathJoinRelX(u.Path)
+ if !strings.HasSuffix(u.Path, "/") {
+ u.Path += "/"
+ }
+ }
+ if urlIsRelative(s, u) {
+ return u.Path == "" || strings.HasPrefix(strings.ToLower(u.Path), strings.ToLower(setting.AppSubURL+"/"))
+ }
+ if u.Path == "" {
+ u.Path = "/"
+ }
+ return strings.HasPrefix(strings.ToLower(u.String()), strings.ToLower(setting.AppURL))
}
diff --git a/modules/httplib/url_test.go b/modules/httplib/url_test.go
index 72033b1208..9b7b242298 100644
--- a/modules/httplib/url_test.go
+++ b/modules/httplib/url_test.go
@@ -7,32 +7,65 @@ 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},
+func TestIsRelativeURL(t *testing.T) {
+ defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")()
+ defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
+ rel := []string{
+ "",
+ "foo",
+ "/",
+ "/foo?k=%20#abc",
+ }
+ for _, s := range rel {
+ assert.True(t, IsRelativeURL(s), "rel = %q", s)
+ }
+ abs := []string{
+ "//",
+ "\\\\",
+ "/\\",
+ "\\/",
+ "mailto:a@b.com",
+ "https://test.com",
+ }
+ for _, s := range abs {
+ assert.False(t, IsRelativeURL(s), "abs = %q", s)
+ }
+}
- {"//", true},
- {"\\\\", true},
- {"/\\", true},
- {"\\/", true},
- {"mail:a@b.com", true},
- {"https://test.com", true},
- {setting.AppURL + "/foo", false},
- }
- for _, tt := range tests {
- t.Run(tt.input, func(t *testing.T) {
- assert.Equal(t, tt.want, IsRiskyRedirectURL(tt.input))
- })
+func TestIsCurrentGiteaSiteURL(t *testing.T) {
+ defer test.MockVariableValue(&setting.AppURL, "http://localhost:3000/sub/")()
+ defer test.MockVariableValue(&setting.AppSubURL, "/sub")()
+ good := []string{
+ "?key=val",
+ "/sub",
+ "/sub/",
+ "/sub/foo",
+ "/sub/foo/",
+ "http://localhost:3000/sub?key=val",
+ "http://localhost:3000/sub/",
}
+ for _, s := range good {
+ assert.True(t, IsCurrentGiteaSiteURL(s), "good = %q", s)
+ }
+ bad := []string{
+ "/",
+ "//",
+ "\\\\",
+ "/foo",
+ "http://localhost:3000/sub/..",
+ "http://localhost:3000/other",
+ "http://other/",
+ }
+ for _, s := range bad {
+ assert.False(t, IsCurrentGiteaSiteURL(s), "bad = %q", s)
+ }
+
+ setting.AppURL = "http://localhost:3000/"
+ setting.AppSubURL = ""
+ assert.True(t, IsCurrentGiteaSiteURL("http://localhost:3000?key=val"))
}
diff --git a/routers/common/redirect.go b/routers/common/redirect.go
index 9bf2025e19..34044e814b 100644
--- a/routers/common/redirect.go
+++ b/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
}
diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go
index da6bef207a..ab81740e3f 100644
--- a/routers/web/auth/auth.go
+++ b/routers/web/auth/auth.go
@@ -133,7 +133,7 @@ func RedirectAfterLogin(ctx *context.Context) {
if setting.LandingPageURL == setting.LandingPageLogin {
nextRedirectTo = setting.AppSubURL + "/" // do not cycle-redirect to the login page
}
- ctx.RedirectToFirst(redirectTo, nextRedirectTo)
+ ctx.RedirectToCurrentSite(redirectTo, nextRedirectTo)
}
func CheckAutoLogin(ctx *context.Context) bool {
@@ -371,7 +371,7 @@ func handleSignInFull(ctx *context.Context, u *user_model.User, remember, obeyRe
if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) {
middleware.DeleteRedirectToCookie(ctx.Resp)
if obeyRedirect {
- ctx.RedirectToFirst(redirectTo)
+ ctx.RedirectToCurrentSite(redirectTo)
}
return redirectTo
}
@@ -808,7 +808,7 @@ func handleAccountActivation(ctx *context.Context, user *user_model.User) {
ctx.Flash.Success(ctx.Tr("auth.account_activated"))
if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 {
middleware.DeleteRedirectToCookie(ctx.Resp)
- ctx.RedirectToFirst(redirectTo)
+ ctx.RedirectToCurrentSite(redirectTo)
return
}
diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go
index d5ca7397f0..3189d1372e 100644
--- a/routers/web/auth/oauth.go
+++ b/routers/web/auth/oauth.go
@@ -1157,7 +1157,7 @@ func handleOAuth2SignIn(ctx *context.Context, source *auth.Source, u *user_model
if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 {
middleware.DeleteRedirectToCookie(ctx.Resp)
- ctx.RedirectToFirst(redirectTo)
+ ctx.RedirectToCurrentSite(redirectTo)
return
}
diff --git a/routers/web/auth/password.go b/routers/web/auth/password.go
index c9e0386041..3af8b7edf2 100644
--- a/routers/web/auth/password.go
+++ b/routers/web/auth/password.go
@@ -314,7 +314,7 @@ func MustChangePasswordPost(ctx *context.Context) {
if redirectTo := ctx.GetSiteCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) {
middleware.DeleteRedirectToCookie(ctx.Resp)
- ctx.RedirectToFirst(redirectTo)
+ ctx.RedirectToCurrentSite(redirectTo)
return
}
diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go
index 7f9bf3210a..0490feb621 100644
--- a/routers/web/repo/repo.go
+++ b/routers/web/repo/repo.go
@@ -371,7 +371,7 @@ func Action(ctx *context.Context) {
return
}
- ctx.RedirectToFirst(ctx.FormString("redirect_to"), ctx.Repo.RepoLink)
+ ctx.RedirectToCurrentSite(ctx.FormString("redirect_to"), ctx.Repo.RepoLink)
}
func acceptOrRejectRepoTransfer(ctx *context.Context, accept bool) error {
diff --git a/routers/web/web.go b/routers/web/web.go
index fc1432873f..3d790d1621 100644
--- a/routers/web/web.go
+++ b/routers/web/web.go
@@ -174,7 +174,7 @@ func verifyAuthWithOptions(options *common.VerifyOptions) func(ctx *context.Cont
// Redirect to dashboard (or alternate location) if user tries to visit any non-login page.
if options.SignOutRequired && ctx.IsSigned && ctx.Req.URL.RequestURI() != "/" {
- ctx.RedirectToFirst(ctx.FormString("redirect_to"))
+ ctx.RedirectToCurrentSite(ctx.FormString("redirect_to"))
return
}
diff --git a/services/context/context_response.go b/services/context/context_response.go
index 372b4cb38b..d7fd18acac 100644
--- a/services/context/context_response.go
+++ b/services/context/context_response.go
@@ -44,14 +44,14 @@ func RedirectToUser(ctx *Base, userName string, redirectUserID int64) {
ctx.Redirect(path.Join(setting.AppSubURL, redirectPath), http.StatusTemporaryRedirect)
}
-// RedirectToFirst redirects to first not empty URL
-func (ctx *Context) RedirectToFirst(location ...string) {
+// RedirectToCurrentSite redirects to first not empty URL which belongs to current site
+func (ctx *Context) RedirectToCurrentSite(location ...string) {
for _, loc := range location {
if len(loc) == 0 {
continue
}
- if httplib.IsRiskyRedirectURL(loc) {
+ if !httplib.IsCurrentGiteaSiteURL(loc) {
continue
}
generated by cgit v1.2.3 (git 2.39.1) at 2025-03-31 05:56:13 +0000