Fix open redirect vulnerability on login screen (#4312)

* 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>
go-gitea · Jun 26, 2018 · 801843b · 801843b
1 parent b8c2420
commit 801843b
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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)