65e83fc · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

on master help sign in

fix(csrf): possible infinite loop (#2607)

65e83fcJohann2 years ago

CHANGELOG.md+1

compare with master

unifiedsplit

@@ -17,6 +17,7 @@

17- [#2734](https://github.com/oauth2-proxy/oauth2-proxy/pull/2734) Added s390x architecture option support (@priby05)

18- [#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)

19- [#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)

21# V7.6.0

22# V7.6.0

pkg/cookies/csrf.go+13−5

@@ -73,15 +73,23 @@ func NewCSRF(opts *options.Cookie, codeVerifier string) (CSRF, error) {

74// LoadCSRFCookie loads a CSRF object from a request's CSRF cookie

75func LoadCSRFCookie(req *http.Request, opts *options.Cookie) (CSRF, error) {

76−

77 cookieName := GenerateCookieName(req, opts)

76 cookieName := GenerateCookieName(req, opts)

78+ cookies := req.Cookies()

79+ for _, cookie := range cookies {

79− cookie, err := req.Cookie(cookieName)

80+ if cookie.Name != cookieName {

81+ continue

82+ }

83+

84+ csrf, err := decodeCSRFCookie(cookie, opts)

80− if err != nil {

85+ if err != nil {

86+ continue

87+ }

88+

81− return nil, err

89+ return csrf, nil

82 }

90 }

84− return decodeCSRFCookie(cookie, opts)

92+ return nil, errors.New("CSRF cookie not found")

85}

93}

87// GenerateCookieName in case cookie options state that CSRF cookie has fixed name then set fixed name, otherwise

95// 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() {

140 Host: cookieDomain,

141 Path: cookiePath,

142 },

143− }

143+ Header: make(http.Header)}

144 })

145

146 AfterEach(func() {

@@ -168,6 +168,67 @@ var _ = Describe("CSRF Cookie Tests", func() {

168 })

169 })

170

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+

171 Context("ClearCookie", func() {

232 Context("ClearCookie", func() {

172 It("sets a cookie with an empty value in the past", func() {

233 It("sets a cookie with an empty value in the past", func() {

173 rw := httptest.NewRecorder()

234 rw := httptest.NewRecorder()