From 801843b0115e29ba2304fa6a5bea1ae169a58e02 Mon Sep 17 00:00:00 2001 From: Jonas Franz Date: Tue, 26 Jun 2018 16:45:18 +0200 Subject: [PATCH] Fix open redirect vulnerability on login screen (#4312) * Fix open redirect vulnerability on login screen Signed-off-by: Jonas Franz * Reorder imports Signed-off-by: Jonas Franz * Replace www. from Domain too Signed-off-by: Jonas Franz --- modules/util/util.go | 13 +++++++++++++ modules/util/util_test.go | 35 +++++++++++++++++++++++++++++++++++ routers/user/auth.go | 3 ++- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/modules/util/util.go b/modules/util/util.go index b6acb9796e..5dcbe448fc 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -10,6 +10,7 @@ import ( "strings" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" ) // OptionalBool a boolean that can be "null" @@ -78,6 +79,18 @@ func URLJoin(base string, elems ...string) string { 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 func Min(a, b int) int { if a > b { diff --git a/modules/util/util_test.go b/modules/util/util_test.go index 0d79df6050..d9357ffa3d 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -7,6 +7,8 @@ package util import ( "testing" + "code.gitea.io/gitea/modules/setting" + "github.com/stretchr/testify/assert" ) @@ -42,3 +44,36 @@ func TestURLJoin(t *testing.T) { 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)) + } +} diff --git a/routers/user/auth.go b/routers/user/auth.go index 9a59f52db2..317b4af3bb 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/go-macaron/captcha" "github.com/markbates/goth" @@ -474,7 +475,7 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR 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) if obeyRedirect { ctx.RedirectToFirst(redirectTo)