Skip to content

Commit b4ac2f3

Browse files
lakhansamanilakhansamani
authored
fix: prevent open redirect in redirect_uri validation (#542)
* fix: prevent open redirect in redirect_uri validation (CVE CWE-601) When allowed_origins=* (the default), IsValidOrigin() accepted any URL as a valid redirect_uri, enabling attackers to steal password reset tokens, magic link tokens, and OAuth tokens by redirecting users to attacker-controlled domains. Add IsValidRedirectURI() that never accepts * as a blanket pass. When allowed_origins=*, only self-origin redirects are allowed. Also add redirect_uri validation to /logout and /authorize which had none. Fixes #540 * test: add wildcard subdomain tests for IsValidRedirectURI Cover *.example.com patterns, port-specific wildcards, multi-origin with wildcards, and bypass attempts like example.com.attacker.com.
1 parent d210298 commit b4ac2f3

12 files changed

Lines changed: 276 additions & 130 deletions

File tree

‎internal/graphql/forgot_password.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ func (g *graphqlProvider) ForgotPassword(ctx context.Context, params *model.Forg
7777
// give higher preference to params redirect uri
7878
if strings.TrimSpace(refs.StringValue(params.RedirectURI)) != "" {
7979
redirectURI = refs.StringValue(params.RedirectURI)
80-
if !validators.IsValidOrigin(redirectURI, g.Config.AllowedOrigins) {
80+
if !validators.IsValidRedirectURI(redirectURI, g.Config.AllowedOrigins, hostname) {
8181
log.Debug().Msg("Invalid redirect URI")
8282
return nil, fmt.Errorf("invalid redirect URI")
8383
}

‎internal/graphql/invite_members.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func (g *graphqlProvider) InviteMembers(ctx context.Context, params *model.Invit
9090
redirectURL := appURL
9191
if params.RedirectURI != nil {
9292
redirectURL = *params.RedirectURI
93-
if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) {
93+
if !validators.IsValidRedirectURI(redirectURL, g.Config.AllowedOrigins, hostname) {
9494
log.Debug().Msg("Invalid redirect URI")
9595
return nil, fmt.Errorf("invalid redirect URI")
9696
}

‎internal/graphql/magic_link_login.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func (g *graphqlProvider) MagicLinkLogin(ctx context.Context, params *model.Magi
150150
redirectURL := parsers.GetAppURL(gc)
151151
if params.RedirectURI != nil {
152152
redirectURL = *params.RedirectURI
153-
if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) {
153+
if !validators.IsValidRedirectURI(redirectURL, g.Config.AllowedOrigins, hostname) {
154154
log.Debug().Msg("Invalid redirect URI")
155155
return nil, fmt.Errorf("invalid redirect URI")
156156
}

‎internal/graphql/signup.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ func (g *graphqlProvider) SignUp(ctx context.Context, params *model.SignUpReques
211211
redirectURL := parsers.GetAppURL(gc)
212212
if params.RedirectURI != nil {
213213
redirectURL = *params.RedirectURI
214-
if !validators.IsValidOrigin(redirectURL, g.Config.AllowedOrigins) {
214+
if !validators.IsValidRedirectURI(redirectURL, g.Config.AllowedOrigins, hostname) {
215215
log.Debug().Msg("Invalid redirect URI")
216216
return nil, fmt.Errorf("invalid redirect URI")
217217
}

‎internal/http_handlers/app.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ func (h *httpProvider) AppHandler() gin.HandlerFunc {
4343
redirectURI = hostname + "/app"
4444
} else {
4545
// validate redirect url with allowed origins
46-
if !validators.IsValidOrigin(redirectURI, h.Config.AllowedOrigins) {
46+
if !validators.IsValidRedirectURI(redirectURI, h.Config.AllowedOrigins, hostname) {
4747
log.Debug().Msg("Invalid redirect url")
4848
c.JSON(400, gin.H{"error": "invalid redirect url"})
4949
return

‎internal/http_handlers/authorize.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import (
4444
"github.com/authorizerdev/authorizer/internal/cookie"
4545
"github.com/authorizerdev/authorizer/internal/parsers"
4646
"github.com/authorizerdev/authorizer/internal/token"
47+
"github.com/authorizerdev/authorizer/internal/validators"
4748
)
4849

4950
// Check the flow for generating and verifying codes: https://developer.okta.com/blog/2019/08/22/okta-authjs-pkce#:~:text=PKCE%20works%20by%20having%20the,is%20called%20the%20Code%20Challenge.
@@ -90,6 +91,16 @@ func (h *httpProvider) AuthorizeHandler() gin.HandlerFunc {
9091

9192
if redirectURI == "" {
9293
redirectURI = "/app"
94+
} else {
95+
hostname := parsers.GetHost(gc)
96+
if !validators.IsValidRedirectURI(redirectURI, h.Config.AllowedOrigins, hostname) {
97+
log.Debug().Msg("Invalid redirect URI")
98+
gc.JSON(http.StatusBadRequest, gin.H{
99+
"error": "invalid_request",
100+
"error_description": "invalid redirect_uri",
101+
})
102+
return
103+
}
93104
}
94105

95106
if responseType == "" {

‎internal/http_handlers/logout.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ import (
1111
"github.com/authorizerdev/authorizer/internal/constants"
1212
"github.com/authorizerdev/authorizer/internal/cookie"
1313
"github.com/authorizerdev/authorizer/internal/crypto"
14+
"github.com/authorizerdev/authorizer/internal/parsers"
1415
"github.com/authorizerdev/authorizer/internal/token"
1516
"github.com/authorizerdev/authorizer/internal/utils"
17+
"github.com/authorizerdev/authorizer/internal/validators"
1618
)
1719

1820
// Handler to logout user
@@ -69,6 +71,14 @@ func (h *httpProvider) LogoutHandler() gin.HandlerFunc {
6971
})
7072

7173
if redirectURL != "" {
74+
hostname := parsers.GetHost(gc)
75+
if !validators.IsValidRedirectURI(redirectURL, h.Config.AllowedOrigins, hostname) {
76+
log.Debug().Msg("Invalid redirect URI")
77+
gc.JSON(http.StatusBadRequest, gin.H{
78+
"error": "invalid redirect uri",
79+
})
80+
return
81+
}
7282
gc.Redirect(http.StatusFound, redirectURL)
7383
} else {
7484
gc.JSON(http.StatusOK, gin.H{

‎internal/http_handlers/oauth_login.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/gin-gonic/gin"
88
"github.com/google/uuid"
99

10+
"github.com/authorizerdev/authorizer/internal/parsers"
1011
"github.com/authorizerdev/authorizer/internal/validators"
1112
)
1213

@@ -31,7 +32,8 @@ func (h *httpProvider) OAuthLoginHandler() gin.HandlerFunc {
3132
return
3233
}
3334

34-
if !validators.IsValidOrigin(redirectURI, h.Config.AllowedOrigins) {
35+
hostname := parsers.GetHost(c)
36+
if !validators.IsValidRedirectURI(redirectURI, h.Config.AllowedOrigins, hostname) {
3537
log.Debug().Msg("Invalid redirect URI")
3638
c.JSON(400, gin.H{
3739
"error": "invalid redirect uri",

‎internal/http_handlers/verify_email.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ import (
2424
func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc {
2525
log := h.Log.With().Str("func", "VerifyEmailHandler").Logger()
2626
return func(c *gin.Context) {
27+
hostname := parsers.GetHost(c)
2728
redirectURL := strings.TrimSpace(c.Query("redirect_uri"))
28-
if redirectURL != "" && !validators.IsValidOrigin(redirectURL, h.Config.AllowedOrigins) {
29+
if redirectURL != "" && !validators.IsValidRedirectURI(redirectURL, h.Config.AllowedOrigins, hostname) {
2930
log.Debug().Msg("Invalid redirect URI")
3031
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid redirect uri"})
3132
return
@@ -49,7 +50,6 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc {
4950
}
5051

5152
// verify if token exists in db
52-
hostname := parsers.GetHost(c)
5353
claim, err := h.TokenProvider.ParseJWTToken(tokenInQuery)
5454
if err != nil {
5555
log.Debug().Err(err).Msg("Error parsing jwt token")
@@ -199,7 +199,7 @@ func (h *httpProvider) VerifyEmailHandler() gin.HandlerFunc {
199199
if redirectURL == "" {
200200
redirectURL = claim["redirect_uri"].(string)
201201
}
202-
if !validators.IsValidOrigin(redirectURL, h.Config.AllowedOrigins) {
202+
if !validators.IsValidRedirectURI(redirectURL, h.Config.AllowedOrigins, hostname) {
203203
log.Debug().Msg("Invalid redirect URI in token claim")
204204
c.JSON(http.StatusBadRequest, gin.H{"error": "invalid redirect uri"})
205205
return

‎internal/integration_tests/redirect_uri_validation_test.go‎

Lines changed: 84 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -1,65 +1,30 @@
11
package integration_tests
22

33
import (
4-
"fmt"
54
"testing"
65

76
"github.com/google/uuid"
87
"github.com/stretchr/testify/assert"
98
"github.com/stretchr/testify/require"
109

11-
"github.com/authorizerdev/authorizer/internal/constants"
12-
"github.com/authorizerdev/authorizer/internal/crypto"
10+
"github.com/authorizerdev/authorizer/internal/config"
1311
"github.com/authorizerdev/authorizer/internal/graph/model"
1412
"github.com/authorizerdev/authorizer/internal/refs"
1513
)
1614

17-
// TestRedirectURIValidation verifies that all endpoints accepting redirect_uri
18-
// validate it against AllowedOrigins to prevent open-redirect token theft.
19-
func TestRedirectURIValidation(t *testing.T) {
20-
cfg := getTestConfig()
21-
cfg.AllowedOrigins = []string{"http://localhost:3000"}
22-
cfg.EnableMagicLinkLogin = true
23-
cfg.EnableEmailVerification = true
24-
cfg.IsEmailServiceEnabled = true
25-
cfg.IsSMSServiceEnabled = true
26-
cfg.SMTPHost = "localhost"
27-
cfg.SMTPPort = 1025
28-
cfg.SMTPSenderEmail = "test@authorizer.dev"
29-
cfg.SMTPSenderName = "Test"
30-
cfg.SMTPLocalName = "Test"
31-
cfg.SMTPSkipTLSVerification = true
32-
33-
ts := initTestSetup(t, cfg)
34-
_, ctx := createContext(ts)
35-
36-
attackerURL := "https://attacker.com/steal"
37-
validURL := "http://localhost:3000/callback"
38-
39-
t.Run("ForgotPassword should reject invalid redirect_uri", func(t *testing.T) {
40-
// First create a user
41-
email := "fp_redirect_test_" + uuid.New().String() + "@authorizer.dev"
42-
password := "Password@123"
43-
signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
44-
Email: &email,
45-
Password: password,
46-
ConfirmPassword: password,
47-
})
48-
require.NoError(t, err)
49-
require.NotNil(t, signupRes)
15+
// TestRedirectURIRejectsAttacker verifies that forgot_password rejects
16+
// attacker-controlled redirect_uri values with explicit AllowedOrigins.
17+
func TestRedirectURIRejectsAttacker(t *testing.T) {
18+
runForEachDB(t, func(t *testing.T, cfg *config.Config) {
19+
cfg.AllowedOrigins = []string{"http://localhost:3000"}
20+
cfg.EnableBasicAuthentication = true
21+
cfg.EnableEmailVerification = false
22+
cfg.EnableSignup = true
5023

51-
// Attacker-controlled redirect_uri should be rejected
52-
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
53-
Email: refs.NewStringRef(email),
54-
RedirectURI: refs.NewStringRef(attackerURL),
55-
})
56-
assert.Error(t, err)
57-
assert.Nil(t, res)
58-
assert.Contains(t, err.Error(), "invalid redirect URI")
59-
})
24+
ts := initTestSetup(t, cfg)
25+
_, ctx := createContext(ts)
6026

61-
t.Run("ForgotPassword should accept valid redirect_uri", func(t *testing.T) {
62-
email := "fp_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
27+
email := "redirect_test_" + uuid.New().String() + "@authorizer.dev"
6328
password := "Password@123"
6429
signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
6530
Email: &email,
@@ -69,98 +34,96 @@ func TestRedirectURIValidation(t *testing.T) {
6934
require.NoError(t, err)
7035
require.NotNil(t, signupRes)
7136

72-
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
73-
Email: refs.NewStringRef(email),
74-
RedirectURI: refs.NewStringRef(validURL),
37+
t.Run("rejects attacker redirect_uri", func(t *testing.T) {
38+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
39+
Email: refs.NewStringRef(email),
40+
RedirectURI: refs.NewStringRef("https://attacker.com/steal"),
41+
})
42+
assert.Error(t, err)
43+
assert.Nil(t, res)
44+
assert.Contains(t, err.Error(), "invalid redirect URI")
7545
})
76-
assert.NoError(t, err)
77-
assert.NotNil(t, res)
78-
})
7946

80-
t.Run("MagicLinkLogin should reject invalid redirect_uri", func(t *testing.T) {
81-
email := "ml_redirect_test_" + uuid.New().String() + "@authorizer.dev"
82-
res, err := ts.GraphQLProvider.MagicLinkLogin(ctx, &model.MagicLinkLoginRequest{
83-
Email: email,
84-
RedirectURI: &attackerURL,
47+
t.Run("accepts valid redirect_uri", func(t *testing.T) {
48+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
49+
Email: refs.NewStringRef(email),
50+
RedirectURI: refs.NewStringRef("http://localhost:3000/reset"),
51+
})
52+
assert.NoError(t, err)
53+
assert.NotNil(t, res)
8554
})
86-
assert.Error(t, err)
87-
assert.Nil(t, res)
88-
assert.Contains(t, err.Error(), "invalid redirect URI")
8955
})
56+
}
9057

91-
t.Run("MagicLinkLogin should accept valid redirect_uri", func(t *testing.T) {
92-
email := "ml_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
93-
res, err := ts.GraphQLProvider.MagicLinkLogin(ctx, &model.MagicLinkLoginRequest{
94-
Email: email,
95-
RedirectURI: &validURL,
96-
})
97-
assert.NoError(t, err)
98-
assert.NotNil(t, res)
99-
})
58+
// TestRedirectURIWildcardOrigins is a regression test for the open redirect
59+
// vulnerability (issue #540). When allowed_origins=["*"] (the default config),
60+
// attacker-controlled redirect_uri values must still be rejected.
61+
func TestRedirectURIWildcardOrigins(t *testing.T) {
62+
runForEachDB(t, func(t *testing.T, cfg *config.Config) {
63+
cfg.AllowedOrigins = []string{"*"}
64+
cfg.EnableBasicAuthentication = true
65+
cfg.EnableEmailVerification = false
66+
cfg.EnableSignup = true
67+
ts := initTestSetup(t, cfg)
68+
_, ctx := createContext(ts)
10069

101-
t.Run("Signup should reject invalid redirect_uri", func(t *testing.T) {
102-
email := "signup_redirect_test_" + uuid.New().String() + "@authorizer.dev"
70+
email := "wildcard_redirect_" + uuid.New().String() + "@authorizer.dev"
10371
password := "Password@123"
104-
res, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
105-
Email: &email,
106-
Password: password,
107-
ConfirmPassword: password,
108-
RedirectURI: &attackerURL,
109-
})
110-
assert.Error(t, err)
111-
assert.Nil(t, res)
112-
assert.Contains(t, err.Error(), "invalid redirect URI")
113-
})
11472

115-
t.Run("Signup should accept valid redirect_uri", func(t *testing.T) {
116-
email := "signup_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
117-
password := "Password@123"
118-
res, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
73+
signupRes, err := ts.GraphQLProvider.SignUp(ctx, &model.SignUpRequest{
11974
Email: &email,
12075
Password: password,
12176
ConfirmPassword: password,
122-
RedirectURI: &validURL,
12377
})
124-
assert.NoError(t, err)
125-
assert.NotNil(t, res)
126-
})
127-
128-
t.Run("InviteMembers should reject invalid redirect_uri", func(t *testing.T) {
129-
cfg.IsEmailServiceEnabled = true
130-
cfg.EnableBasicAuthentication = true
131-
cfg.EnableMagicLinkLogin = true
132-
133-
req, _ := createContext(ts)
134-
h, err := crypto.EncryptPassword(cfg.AdminSecret)
13578
require.NoError(t, err)
136-
req.Header.Set("Cookie", fmt.Sprintf("%s=%s", constants.AdminCookieName, h))
79+
require.NotNil(t, signupRes)
13780

138-
emailTo := "invite_redirect_test_" + uuid.New().String() + "@authorizer.dev"
139-
res, err := ts.GraphQLProvider.InviteMembers(ctx, &model.InviteMemberRequest{
140-
Emails: []string{emailTo},
141-
RedirectURI: &attackerURL,
81+
t.Run("rejects attacker redirect_uri with wildcard origins", func(t *testing.T) {
82+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
83+
Email: refs.NewStringRef(email),
84+
RedirectURI: refs.NewStringRef("https://attacker.com/capture"),
85+
})
86+
assert.Error(t, err)
87+
assert.Nil(t, res)
88+
assert.Contains(t, err.Error(), "invalid redirect URI")
14289
})
143-
assert.Error(t, err)
144-
assert.Nil(t, res)
145-
assert.Contains(t, err.Error(), "invalid redirect URI")
146-
})
14790

148-
t.Run("InviteMembers should accept valid redirect_uri", func(t *testing.T) {
149-
cfg.IsEmailServiceEnabled = true
150-
cfg.EnableBasicAuthentication = true
151-
cfg.EnableMagicLinkLogin = true
91+
t.Run("allows self-origin redirect_uri with wildcard origins", func(t *testing.T) {
92+
selfURI := "http://" + ts.HttpServer.Listener.Addr().String() + "/app/reset-password"
93+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
94+
Email: refs.NewStringRef(email),
95+
RedirectURI: refs.NewStringRef(selfURI),
96+
})
97+
assert.NoError(t, err)
98+
assert.NotNil(t, res)
99+
assert.NotEmpty(t, res.Message)
100+
})
152101

153-
req, _ := createContext(ts)
154-
h, err := crypto.EncryptPassword(cfg.AdminSecret)
155-
require.NoError(t, err)
156-
req.Header.Set("Cookie", fmt.Sprintf("%s=%s", constants.AdminCookieName, h))
102+
t.Run("works without redirect_uri (uses default)", func(t *testing.T) {
103+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
104+
Email: refs.NewStringRef(email),
105+
})
106+
assert.NoError(t, err)
107+
assert.NotNil(t, res)
108+
assert.NotEmpty(t, res.Message)
109+
})
110+
111+
t.Run("rejects javascript scheme", func(t *testing.T) {
112+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
113+
Email: refs.NewStringRef(email),
114+
RedirectURI: refs.NewStringRef("javascript:alert(1)"),
115+
})
116+
assert.Error(t, err)
117+
assert.Nil(t, res)
118+
})
157119

158-
emailTo := "invite_valid_redirect_" + uuid.New().String() + "@authorizer.dev"
159-
res, err := ts.GraphQLProvider.InviteMembers(ctx, &model.InviteMemberRequest{
160-
Emails: []string{emailTo},
161-
RedirectURI: &validURL,
120+
t.Run("rejects data scheme", func(t *testing.T) {
121+
res, err := ts.GraphQLProvider.ForgotPassword(ctx, &model.ForgotPasswordRequest{
122+
Email: refs.NewStringRef(email),
123+
RedirectURI: refs.NewStringRef("data:text/html,<h1>evil</h1>"),
124+
})
125+
assert.Error(t, err)
126+
assert.Nil(t, res)
162127
})
163-
assert.NoError(t, err)
164-
assert.NotNil(t, res)
165128
})
166129
}

0 commit comments

Comments
 (0)