Skip to content

Commit 749fb23

Browse files
committed
fix(gauth): prevent prefix-based bypass attacks
1 parent 58b5e4a commit 749fb23

File tree

1 file changed

+40
-24
lines changed

1 file changed

+40
-24
lines changed

httpapi/auth/gauth.go

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"net/http"
77
"net/url"
8-
"strings"
98

109
"github.com/database-playground/backend-v2/internal/auth"
1110
"github.com/database-playground/backend-v2/internal/authutil"
@@ -170,7 +169,18 @@ func (h *GauthHandler) Callback(c *gin.Context) {
170169
return
171170
}
172171

173-
// get redirect URL from cookie
172+
// write to cookie
173+
c.SetCookie(
174+
/* name */ auth.CookieAuthToken,
175+
/* value */ token,
176+
/* maxAge */ auth.DefaultTokenExpire,
177+
/* path */ "/",
178+
/* domain */ "",
179+
/* secure */ true,
180+
/* httpOnly */ true,
181+
)
182+
183+
// redirect to the original redirect URL
174184
redirectURL, err := c.Cookie(redirectCookieName)
175185
if err != nil {
176186
if errors.Is(err, http.ErrNoCookie) {
@@ -188,31 +198,37 @@ func (h *GauthHandler) Callback(c *gin.Context) {
188198
}
189199

190200
// check if the redirect URL is in the allowed redirect URIs
191-
for _, allowedRedirectURI := range h.redirectURIs {
192-
if strings.HasPrefix(redirectURL, allowedRedirectURI) {
193-
redirectURL = allowedRedirectURI
194-
break
195-
}
196-
}
197-
198-
if redirectURL == "" {
199-
c.JSON(http.StatusBadRequest, gin.H{
200-
"error": "redirect URL is not allowed",
201-
"detail": fmt.Sprintf("redirect URL is not allowed: %s", redirectURL),
201+
userRedirectURL, err := url.Parse(redirectURL)
202+
if err != nil {
203+
c.JSON(http.StatusInternalServerError, gin.H{
204+
"error": "failed to parse redirect URL",
205+
"detail": err.Error(),
202206
})
203207
return
204208
}
205209

206-
// write to cookie
207-
c.SetCookie(
208-
/* name */ auth.CookieAuthToken,
209-
/* value */ token,
210-
/* maxAge */ auth.DefaultTokenExpire,
211-
/* path */ "/",
212-
/* domain */ "",
213-
/* secure */ true,
214-
/* httpOnly */ true,
215-
)
210+
for _, allowedRedirectURI := range h.redirectURIs {
211+
parsedAllowedRedirectURI, err := url.Parse(allowedRedirectURI)
212+
if err != nil {
213+
c.JSON(http.StatusInternalServerError, gin.H{
214+
"error": "failed to parse allowed redirect URI",
215+
"detail": err.Error(),
216+
})
217+
return
218+
}
216219

217-
c.Redirect(http.StatusTemporaryRedirect, redirectURL)
220+
matched := userRedirectURL.Scheme == parsedAllowedRedirectURI.Scheme &&
221+
userRedirectURL.Host == parsedAllowedRedirectURI.Host &&
222+
userRedirectURL.Path == parsedAllowedRedirectURI.Path
223+
224+
if matched {
225+
c.Redirect(http.StatusTemporaryRedirect, parsedAllowedRedirectURI.String())
226+
return
227+
}
228+
}
229+
230+
c.JSON(http.StatusBadRequest, gin.H{
231+
"error": "redirect URL is not allowed",
232+
"detail": fmt.Sprintf("redirect URL is not allowed: %s", redirectURL),
233+
})
218234
}

0 commit comments

Comments
 (0)