Skip to content

Commit 347e23a

Browse files
authored
fix: redirects must not be to ip addresses (#1984)
Properly fixes the attempt in #1974
1 parent 8b23382 commit 347e23a

File tree

2 files changed

+41
-5
lines changed

2 files changed

+41
-5
lines changed

internal/utilities/request.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net"
77
"net/http"
88
"net/url"
9+
"regexp"
910
"strings"
1011

1112
"github.com/supabase/auth/internal/conf"
@@ -79,6 +80,8 @@ func GetReferrer(r *http.Request, config *conf.GlobalConfiguration) string {
7980
return config.SiteURL
8081
}
8182

83+
var decimalIPAddressPattern = regexp.MustCompile("^[0-9]+$")
84+
8285
func IsRedirectURLValid(config *conf.GlobalConfiguration, redirectURL string) bool {
8386
if redirectURL == "" {
8487
return false
@@ -92,9 +95,17 @@ func IsRedirectURLValid(config *conf.GlobalConfiguration, redirectURL string) bo
9295
return true
9396
}
9497

95-
// Clean up the referrer URL to avoid pattern matching an invalid URL
96-
refurl.Fragment = ""
97-
refurl.RawQuery = ""
98+
if rerr != nil {
99+
// redirect URL is for some reason invalid
100+
return false
101+
}
102+
103+
if decimalIPAddressPattern.MatchString(refurl.Hostname()) {
104+
// IP address in decimal form also not allowed in redirects!
105+
return false
106+
} else if ip := net.ParseIP(refurl.Hostname()); ip != nil {
107+
return ip.IsLoopback()
108+
}
98109

99110
// For case when user came from mobile app or other permitted resource - redirect back
100111
for _, pattern := range config.URIAllowListMap {

internal/utilities/request_test.go

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ func TestGetIPAddress(t *tst.T) {
9191
func TestGetReferrer(t *tst.T) {
9292
config := conf.GlobalConfiguration{
9393
SiteURL: "https://example.com",
94-
URIAllowList: []string{"http://localhost:8000/*", "http://*.localhost:8000/*"},
94+
URIAllowList: []string{"http://localhost:8000/*", "http://*.localhost:8000/*", "http://*:12345/*", "http://**:12345/*"},
9595
JWT: conf.JWTConfiguration{
9696
Secret: "testsecret",
9797
},
@@ -128,10 +128,35 @@ func TestGetReferrer(t *tst.T) {
128128
expected: "http://localhost:8000/path?param=1",
129129
},
130130
{
131-
desc: "invalid redirect url via query smurfing",
131+
desc: "invalid redirect url due to decimal IP address",
132132
redirectURL: "http://123?.localhost:8000/path",
133133
expected: config.SiteURL,
134134
},
135+
{
136+
desc: "invalid redirect url due to IPv4 address",
137+
redirectURL: "http://123.123.123.123?localhost:8000/path",
138+
expected: config.SiteURL,
139+
},
140+
{
141+
desc: "invalid redirect url due to IPv6 address",
142+
redirectURL: "http://[65e7:9410:d8b6:e227:58cd:e55b:8fc0:206d]?localhost:8000/path",
143+
expected: config.SiteURL,
144+
},
145+
{
146+
desc: "invalid redirect url due to bad URL",
147+
redirectURL: "http://65e7:9410:d8b6:e227:58cd:e55b:8fc0:206d?localhost:8000/path",
148+
expected: config.SiteURL,
149+
},
150+
{
151+
desc: "valid loopback IPv4 address",
152+
redirectURL: "http://127.0.0.1:12345/path",
153+
expected: "http://127.0.0.1:12345/path",
154+
},
155+
{
156+
desc: "valid loopback IPv6 address",
157+
redirectURL: "http://[0:0:0:0:0:0:0:1]:12345/path",
158+
expected: "http://[0:0:0:0:0:0:0:1]:12345/path",
159+
},
135160
}
136161

137162
for _, c := range cases {

0 commit comments

Comments
 (0)