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>
This commit is contained in:
		
							parent
							
								
									b8c2420ae2
								
							
						
					
					
						commit
						801843b011
					
				
					 3 changed files with 50 additions and 1 deletions
				
			
		|  | @ -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) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in a new issue