* Fix open redirect vulnerability on login screen Signed-off-by: Jonas Franz <info@jonasfranz.software> * Reorder imports Signed-off-by: Jonas Franz <info@jonasfranz.software> * Replace www. from Domain too Signed-off-by: Jonas Franz <info@jonasfranz.software>tags/v1.5.0-dev
| @@ -10,6 +10,7 @@ import ( | |||||
| "strings" | "strings" | ||||
| "code.gitea.io/gitea/modules/log" | "code.gitea.io/gitea/modules/log" | ||||
| "code.gitea.io/gitea/modules/setting" | |||||
| ) | ) | ||||
| // OptionalBool a boolean that can be "null" | // OptionalBool a boolean that can be "null" | ||||
| @@ -78,6 +79,18 @@ func URLJoin(base string, elems ...string) string { | |||||
| return joinedURL | return joinedURL | ||||
| } | } | ||||
| // IsExternalURL checks if rawURL points to an external URL like http://example.com | |||||
| func IsExternalURL(rawURL string) bool { | |||||
| parsed, err := url.Parse(rawURL) | |||||
| if err != nil { | |||||
| return true | |||||
| } | |||||
| if len(parsed.Host) != 0 && strings.Replace(parsed.Host, "www.", "", 1) != strings.Replace(setting.Domain, "www.", "", 1) { | |||||
| return true | |||||
| } | |||||
| return false | |||||
| } | |||||
| // Min min of two ints | // Min min of two ints | ||||
| func Min(a, b int) int { | func Min(a, b int) int { | ||||
| if a > b { | if a > b { | ||||
| @@ -7,6 +7,8 @@ package util | |||||
| import ( | import ( | ||||
| "testing" | "testing" | ||||
| "code.gitea.io/gitea/modules/setting" | |||||
| "github.com/stretchr/testify/assert" | "github.com/stretchr/testify/assert" | ||||
| ) | ) | ||||
| @@ -42,3 +44,36 @@ func TestURLJoin(t *testing.T) { | |||||
| assert.Equal(t, test.Expected, URLJoin(test.Base, test.Elements...)) | assert.Equal(t, test.Expected, URLJoin(test.Base, test.Elements...)) | ||||
| } | } | ||||
| } | } | ||||
| func TestIsExternalURL(t *testing.T) { | |||||
| setting.Domain = "try.gitea.io" | |||||
| type test struct { | |||||
| Expected bool | |||||
| RawURL string | |||||
| } | |||||
| newTest := func(expected bool, rawURL string) test { | |||||
| return test{Expected: expected, RawURL: rawURL} | |||||
| } | |||||
| for _, test := range []test{ | |||||
| newTest(false, | |||||
| "https://try.gitea.io"), | |||||
| newTest(true, | |||||
| "https://example.com/"), | |||||
| newTest(true, | |||||
| "//example.com"), | |||||
| newTest(true, | |||||
| "http://example.com"), | |||||
| newTest(false, | |||||
| "a/"), | |||||
| newTest(false, | |||||
| "https://try.gitea.io/test?param=false"), | |||||
| newTest(false, | |||||
| "test?param=false"), | |||||
| newTest(false, | |||||
| "//try.gitea.io/test?param=false"), | |||||
| newTest(false, | |||||
| "/hey/hey/hey#3244"), | |||||
| } { | |||||
| assert.Equal(t, test.Expected, IsExternalURL(test.RawURL)) | |||||
| } | |||||
| } | |||||
| @@ -18,6 +18,7 @@ import ( | |||||
| "code.gitea.io/gitea/modules/context" | "code.gitea.io/gitea/modules/context" | ||||
| "code.gitea.io/gitea/modules/log" | "code.gitea.io/gitea/modules/log" | ||||
| "code.gitea.io/gitea/modules/setting" | "code.gitea.io/gitea/modules/setting" | ||||
| "code.gitea.io/gitea/modules/util" | |||||
| "github.com/go-macaron/captcha" | "github.com/go-macaron/captcha" | ||||
| "github.com/markbates/goth" | "github.com/markbates/goth" | ||||
| @@ -474,7 +475,7 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR | |||||
| return setting.AppSubURL + "/" | return setting.AppSubURL + "/" | ||||
| } | } | ||||
| if redirectTo, _ := url.QueryUnescape(ctx.GetCookie("redirect_to")); len(redirectTo) > 0 { | |||||
| if redirectTo, _ := url.QueryUnescape(ctx.GetCookie("redirect_to")); len(redirectTo) > 0 && !util.IsExternalURL(redirectTo) { | |||||
| ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) | ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) | ||||
| if obeyRedirect { | if obeyRedirect { | ||||
| ctx.RedirectToFirst(redirectTo) | ctx.RedirectToFirst(redirectTo) | ||||