fix: runtime error: index out of range (0) with length 0 (#2328)
* Issue 2311: runtime error: index out of range [0] with length 0 while extracting state of of the csrf --------- Co-authored-by: tuunit <jan@larwig.com>
This commit is contained in:
parent
642ba174d4
commit
ff761d2523
@ -10,6 +10,7 @@
|
||||
|
||||
- [#2803](https://github.com/oauth2-proxy/oauth2-proxy/pull/2803) fix: self signed certificate handling in v7.7.0 (@tuunit)
|
||||
- [#2619](https://github.com/oauth2-proxy/oauth2-proxy/pull/2619) fix: unable to use hyphen in JSON path for oidc-groups-claim option (@rd-danny-fleer)
|
||||
- [#2311](https://github.com/oauth2-proxy/oauth2-proxy/pull/2311) fix: runtime error: index out of range (0) with length 0 (@miguelborges99 / @tuunit)
|
||||
|
||||
# V7.7.0
|
||||
|
||||
|
@ -870,9 +870,22 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
csrf, err := cookies.LoadCSRFCookie(req, p.CookieOptions)
|
||||
nonce, appRedirect, err := decodeState(req.Form.Get("state"), p.encodeState)
|
||||
if err != nil {
|
||||
logger.Println(req, logger.AuthFailure, "Invalid authentication via OAuth2. Error while loading CSRF cookie:", err.Error())
|
||||
logger.Errorf("Error while parsing OAuth2 state: %v", err)
|
||||
p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
// calculate the cookie name
|
||||
cookieName := cookies.GenerateCookieName(p.CookieOptions, nonce)
|
||||
// Try to find the CSRF cookie and decode it
|
||||
csrf, err := cookies.LoadCSRFCookie(req, cookieName, p.CookieOptions)
|
||||
if err != nil {
|
||||
// There are a lot of issues opened complaining about missing CSRF cookies.
|
||||
// Try to log the INs and OUTs of OAuthProxy, to be easier to analyse these issues.
|
||||
LoggingCSRFCookiesInOAuthCallback(req, cookieName)
|
||||
logger.Println(req, logger.AuthFailure, "Invalid authentication via OAuth2: unable to obtain CSRF cookie: %s (state=%s)", err, nonce)
|
||||
p.ErrorPage(rw, req, http.StatusForbidden, err.Error(), "Login Failed: Unable to find a valid CSRF token. Please try again.")
|
||||
return
|
||||
}
|
||||
@ -893,13 +906,6 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
|
||||
|
||||
csrf.ClearCookie(rw, req)
|
||||
|
||||
nonce, appRedirect, err := decodeState(req.Form.Get("state"), p.encodeState)
|
||||
if err != nil {
|
||||
logger.Errorf("Error while parsing OAuth2 state: %v", err)
|
||||
p.ErrorPage(rw, req, http.StatusInternalServerError, err.Error())
|
||||
return
|
||||
}
|
||||
|
||||
if !csrf.CheckOAuthState(nonce) {
|
||||
logger.PrintAuthf(session.Email, req, logger.AuthFailure, "Invalid authentication via OAuth2: CSRF token mismatch, potential attack")
|
||||
p.ErrorPage(rw, req, http.StatusForbidden, "CSRF token mismatch, potential attack", "Login Failed: Unable to find a valid CSRF token. Please try again.")
|
||||
@ -1297,3 +1303,27 @@ func (p *OAuthProxy) errorJSON(rw http.ResponseWriter, code int) {
|
||||
// application/json
|
||||
rw.Write([]byte("{}"))
|
||||
}
|
||||
|
||||
// LoggingCSRFCookiesInOAuthCallback Log all CSRF cookies found in HTTP request OAuth callback,
|
||||
// which were successfully parsed
|
||||
func LoggingCSRFCookiesInOAuthCallback(req *http.Request, cookieName string) {
|
||||
cookies := req.Cookies()
|
||||
if len(cookies) == 0 {
|
||||
logger.Println(req, logger.AuthFailure, "No cookies were found in OAuth callback.")
|
||||
return
|
||||
}
|
||||
|
||||
for _, c := range cookies {
|
||||
if cookieName == c.Name {
|
||||
logger.Println(req, logger.AuthFailure, "CSRF cookie %s was found in OAuth callback.", c.Name)
|
||||
return
|
||||
}
|
||||
|
||||
if strings.HasSuffix(c.Name, "_csrf") {
|
||||
logger.Println(req, logger.AuthFailure, "CSRF cookie %s was found in OAuth callback, but it is not the expected one (%s).", c.Name, cookieName)
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
logger.Println(req, logger.AuthFailure, "Cookies were found in OAuth callback, but none was a CSRF cookie.")
|
||||
}
|
||||
|
@ -72,9 +72,7 @@ func NewCSRF(opts *options.Cookie, codeVerifier string) (CSRF, error) {
|
||||
}
|
||||
|
||||
// LoadCSRFCookie loads a CSRF object from a request's CSRF cookie
|
||||
func LoadCSRFCookie(req *http.Request, opts *options.Cookie) (CSRF, error) {
|
||||
cookieName := GenerateCookieName(req, opts)
|
||||
|
||||
func LoadCSRFCookie(req *http.Request, cookieName string, opts *options.Cookie) (CSRF, error) {
|
||||
cookies := req.Cookies()
|
||||
for _, cookie := range cookies {
|
||||
if cookie.Name != cookieName {
|
||||
@ -89,17 +87,17 @@ func LoadCSRFCookie(req *http.Request, opts *options.Cookie) (CSRF, error) {
|
||||
return csrf, nil
|
||||
}
|
||||
|
||||
return nil, errors.New("CSRF cookie not found")
|
||||
return nil, fmt.Errorf("CSRF cookie with name '%v' was not found", cookieName)
|
||||
}
|
||||
|
||||
// GenerateCookieName in case cookie options state that CSRF cookie has fixed name then set fixed name, otherwise
|
||||
// build name based on the state
|
||||
func GenerateCookieName(req *http.Request, opts *options.Cookie) string {
|
||||
func GenerateCookieName(opts *options.Cookie, state string) string {
|
||||
stateSubstring := ""
|
||||
if opts.CSRFPerRequest {
|
||||
// csrfCookieName will include a substring of the state to enable multiple csrf cookies
|
||||
// in case of parallel requests
|
||||
stateSubstring = ExtractStateSubstring(req)
|
||||
stateSubstring = ExtractStateSubstring(state)
|
||||
}
|
||||
return csrfCookieName(opts, stateSubstring)
|
||||
}
|
||||
@ -218,21 +216,16 @@ func csrfCookieName(opts *options.Cookie, stateSubstring string) string {
|
||||
if stateSubstring == "" {
|
||||
return fmt.Sprintf("%v_csrf", opts.Name)
|
||||
}
|
||||
return fmt.Sprintf("%v_csrf_%v", opts.Name, stateSubstring)
|
||||
return fmt.Sprintf("%v_%v_csrf", opts.Name, stateSubstring)
|
||||
}
|
||||
|
||||
// ExtractStateSubstring extract the initial state characters, to add it to the CSRF cookie name
|
||||
func ExtractStateSubstring(req *http.Request) string {
|
||||
func ExtractStateSubstring(state string) string {
|
||||
lastChar := csrfStateLength - 1
|
||||
stateSubstring := ""
|
||||
|
||||
state := req.URL.Query()["state"]
|
||||
if state[0] != "" {
|
||||
state := state[0]
|
||||
if lastChar <= len(state) {
|
||||
stateSubstring = state[0:lastChar]
|
||||
}
|
||||
}
|
||||
return stateSubstring
|
||||
}
|
||||
|
||||
|
@ -19,6 +19,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
|
||||
cookieOpts *options.Cookie
|
||||
publicCSRF CSRF
|
||||
privateCSRF *csrf
|
||||
csrfName string
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
@ -39,6 +40,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
|
||||
privateCSRF = publicCSRF.(*csrf)
|
||||
csrfName = GenerateCookieName(cookieOpts, csrfNonce)
|
||||
})
|
||||
|
||||
Context("NewCSRF", func() {
|
||||
@ -175,7 +177,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
|
||||
})
|
||||
|
||||
It("should return error when no cookie is set", func() {
|
||||
csrf, err := LoadCSRFCookie(req, cookieOpts)
|
||||
csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(csrf).To(BeNil())
|
||||
})
|
||||
@ -191,7 +193,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
|
||||
Value: encoded,
|
||||
})
|
||||
|
||||
csrf, err := LoadCSRFCookie(req, cookieOpts)
|
||||
csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(csrf).ToNot(BeNil())
|
||||
})
|
||||
@ -202,7 +204,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
|
||||
Value: "invalid",
|
||||
})
|
||||
|
||||
csrf, err := LoadCSRFCookie(req, cookieOpts)
|
||||
csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)
|
||||
Expect(err).To(HaveOccurred())
|
||||
Expect(csrf).To(BeNil())
|
||||
})
|
||||
@ -223,7 +225,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
|
||||
Value: encoded,
|
||||
})
|
||||
|
||||
csrf, err := LoadCSRFCookie(req, cookieOpts)
|
||||
csrf, err := LoadCSRFCookie(req, csrfName, cookieOpts)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(csrf).ToNot(BeNil())
|
||||
})
|
||||
|
Loading…
x
Reference in New Issue
Block a user