git.iko.soy/typeable/oauth2-proxy
on master help sign in
fix(csrf): possible infinite loop (#2607)
commit65e83fc3ccba2b11444d109e07c997e423d485c6 authorJohann <76482511+Primexz@users.noreply.github.com> dateMon 30 Sep 2024, 18:20 (2 years ago) parent021d940
3 files changed+76−6
compare with master
unifiedsplit
CHANGELOG.md+1 pkg/cookies/csrf.go+13−5 pkg/cookies/csrf_test.go+62−1
CHANGELOG.md+1
@@ -17,6 +17,7 @@
1717- [#2734](https://github.com/oauth2-proxy/oauth2-proxy/pull/2734) Added s390x architecture option support (@priby05)
1818- [#2589](https://github.com/oauth2-proxy/oauth2-proxy/pull/2589) Added support for regex path matching and rewriting when using a static `file:` upstream (@ianroberts)
1919- [#2790](https://github.com/oauth2-proxy/oauth2-proxy/pull/2790) chore(deps): update all golang dependencies (@tuunit)
20+- [#2607](https://github.com/oauth2-proxy/oauth2-proxy/pull/2607) fix(csrf): fix possible infinite loop (@Primexz)
2021
2122# V7.6.0
2223
pkg/cookies/csrf.go+13−5
@@ -73,15 +73,23 @@ func NewCSRF(opts *options.Cookie, codeVerifier string) (CSRF, error) {
7373
7474// LoadCSRFCookie loads a CSRF object from a request's CSRF cookie
7575func LoadCSRFCookie(req *http.Request, opts *options.Cookie) (CSRF, error) {
76−
7776 cookieName := GenerateCookieName(req, opts)
7877
79− cookie, err := req.Cookie(cookieName)
80− if err != nil {
81− return nil, err
78+ cookies := req.Cookies()
79+ for _, cookie := range cookies {
80+ if cookie.Name != cookieName {
81+ continue
82+ }
83+
84+ csrf, err := decodeCSRFCookie(cookie, opts)
85+ if err != nil {
86+ continue
87+ }
88+
89+ return csrf, nil
8290 }
8391
84− return decodeCSRFCookie(cookie, opts)
92+ return nil, errors.New("CSRF cookie not found")
8593}
8694
8795// GenerateCookieName in case cookie options state that CSRF cookie has fixed name then set fixed name, otherwise
pkg/cookies/csrf_test.go+62−1
@@ -140,7 +140,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
140140 Host: cookieDomain,
141141 Path: cookiePath,
142142 },
143− }
143+ Header: make(http.Header)}
144144 })
145145
146146 AfterEach(func() {
@@ -168,6 +168,67 @@ var _ = Describe("CSRF Cookie Tests", func() {
168168 })
169169 })
170170
171+ Context("LoadCSRFCookie", func() {
172+ BeforeEach(func() {
173+ // we need to reset the time to ensure the cookie is valid
174+ privateCSRF.time.Reset()
175+ })
176+
177+ It("should return error when no cookie is set", func() {
178+ csrf, err := LoadCSRFCookie(req, cookieOpts)
179+ Expect(err).To(HaveOccurred())
180+ Expect(csrf).To(BeNil())
181+ })
182+
183+ It("should find one valid cookie", func() {
184+ privateCSRF.OAuthState = []byte(csrfState)
185+ privateCSRF.OIDCNonce = []byte(csrfNonce)
186+ encoded, err := privateCSRF.encodeCookie()
187+ Expect(err).ToNot(HaveOccurred())
188+
189+ req.AddCookie(&http.Cookie{
190+ Name: privateCSRF.cookieName(),
191+ Value: encoded,
192+ })
193+
194+ csrf, err := LoadCSRFCookie(req, cookieOpts)
195+ Expect(err).ToNot(HaveOccurred())
196+ Expect(csrf).ToNot(BeNil())
197+ })
198+
199+ It("should return error when one invalid cookie is set", func() {
200+ req.AddCookie(&http.Cookie{
201+ Name: privateCSRF.cookieName(),
202+ Value: "invalid",
203+ })
204+
205+ csrf, err := LoadCSRFCookie(req, cookieOpts)
206+ Expect(err).To(HaveOccurred())
207+ Expect(csrf).To(BeNil())
208+ })
209+
210+ It("should be able to handle two cookie with one invalid", func() {
211+ privateCSRF.OAuthState = []byte(csrfState)
212+ privateCSRF.OIDCNonce = []byte(csrfNonce)
213+ encoded, err := privateCSRF.encodeCookie()
214+ Expect(err).ToNot(HaveOccurred())
215+
216+ req.AddCookie(&http.Cookie{
217+ Name: privateCSRF.cookieName(),
218+ Value: "invalid",
219+ })
220+
221+ req.AddCookie(&http.Cookie{
222+ Name: privateCSRF.cookieName(),
223+ Value: encoded,
224+ })
225+
226+ csrf, err := LoadCSRFCookie(req, cookieOpts)
227+ Expect(err).ToNot(HaveOccurred())
228+ Expect(csrf).ToNot(BeNil())
229+ })
230+ })
231+
171232 Context("ClearCookie", func() {
172233 It("sets a cookie with an empty value in the past", func() {
173234 rw := httptest.NewRecorder()