Skip to content

Commit 0abba96

Browse files
committed
feat: properly handle redirect url fragments and unusual hostnames
1 parent 7ff716b commit 0abba96

File tree

2 files changed

+22
-1
lines changed

2 files changed

+22
-1
lines changed

internal/api/verify_test.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,20 @@ func (ts *VerifyTestSuite) TestVerifySignupWithRedirectURLContainedPath() {
657657
requestredirectURL: "http://test.dev:3000/bar/foo",
658658
expectedredirectURL: "http://localhost:3000",
659659
},
660+
{
661+
desc: "redirect ignores fragment for validation",
662+
siteURL: "http://localhost:3000",
663+
uriAllowList: []string{"http://*.example.com/*"},
664+
requestredirectURL: "http://something#.example.com/abc",
665+
expectedredirectURL: "http://localhost:3000",
666+
},
667+
{
668+
desc: "redirect ignores unusual hostnames",
669+
siteURL: "http://localhost:3000",
670+
uriAllowList: []string{"http://*.example.com/*"},
671+
requestredirectURL: "http://japanese。.example.com/abc",
672+
expectedredirectURL: "http://localhost:3000",
673+
},
660674
}
661675

662676
for _, tC := range testCases {

internal/utilities/request.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ func GetReferrer(r *http.Request, config *conf.GlobalConfiguration) string {
8181
}
8282

8383
var decimalIPAddressPattern = regexp.MustCompile("^[0-9]+$")
84+
var regularHostname = regexp.MustCompile("^[a-zA-Z0-9]([a-zA-Z0-9.-]*[a-zA-Z0-9])?$")
8485

8586
func IsRedirectURLValid(config *conf.GlobalConfiguration, redirectURL string) bool {
8687
if redirectURL == "" {
@@ -105,11 +106,17 @@ func IsRedirectURLValid(config *conf.GlobalConfiguration, redirectURL string) bo
105106
return false
106107
} else if ip := net.ParseIP(refurl.Hostname()); ip != nil {
107108
return ip.IsLoopback()
109+
} else if !regularHostname.MatchString(refurl.Hostname()) {
110+
// hostname uses characters that are not typically used
111+
return false
108112
}
109113

110114
// For case when user came from mobile app or other permitted resource - redirect back
111115
for _, pattern := range config.URIAllowListMap {
112-
if pattern.Match(redirectURL) {
116+
// only match without the fragment
117+
matchAgainst, _, _ := strings.Cut(redirectURL, "#")
118+
119+
if pattern.Match(matchAgainst) {
113120
return true
114121
}
115122
}

0 commit comments

Comments
 (0)