From e6287362ad000400f8a26f082685dd330ff0e9f2 Mon Sep 17 00:00:00 2001 From: Maximilian Tagher Date: Sun, 10 Jul 2016 21:37:17 -0700 Subject: [PATCH] Default CSRF tokens to the root path "/" * The default path of cookies is the current path making the request * e.g. an AJAX request made from http://example.com/foo/bar would be /foo * This causes multiple CSRF tokens to build up as you navigate a site * This will cause errors if the CSRF tokens have different values, and an invalid token is sent. * Closes #1247 --- yesod-core/ChangeLog.md | 5 +++++ yesod-core/Yesod/Core/Class/Yesod.hs | 6 +++++- yesod-core/Yesod/Core/Handler.hs | 8 ++++++-- yesod-core/test/YesodCoreTest/Csrf.hs | 6 ++++++ 4 files changed, 22 insertions(+), 3 deletions(-) diff --git a/yesod-core/ChangeLog.md b/yesod-core/ChangeLog.md index 281f1d48..621ebd90 100644 --- a/yesod-core/ChangeLog.md +++ b/yesod-core/ChangeLog.md @@ -1,3 +1,8 @@ +## 1.4.23.1 + +* Don't allow sending multiple cookies with the same name to the client, in accordance with [RFC 6265](https://tools.ietf.org/html/rfc6265). This fixes an issue where multiple CSRF tokens were sent to the client. [#1258](https://github.com/yesodweb/yesod/pull/1258) +* Default CSRF tokens to the root path "/", fixing an issue where multiple tokens were stored in cookies, and using the wrong one led to CSRF errors [#1248](https://github.com/yesodweb/yesod/pull/1248) + ## 1.4.23 * urlParamRenderOverride method for Yesod class [#1257](https://github.com/yesodweb/yesod/pull/1257) diff --git a/yesod-core/Yesod/Core/Class/Yesod.hs b/yesod-core/Yesod/Core/Class/Yesod.hs index 792b1d43..3d426982 100644 --- a/yesod-core/Yesod/Core/Class/Yesod.hs +++ b/yesod-core/Yesod/Core/Class/Yesod.hs @@ -491,14 +491,18 @@ csrfCheckMiddleware handler shouldCheckFn headerName paramName = do -- | Calls 'csrfSetCookieMiddleware' with the 'defaultCsrfCookieName'. -- +-- The cookie's path is set to @/@, making it valid for your whole website. +-- -- Since 1.4.14 defaultCsrfSetCookieMiddleware :: Yesod site => HandlerT site IO res -> HandlerT site IO res -defaultCsrfSetCookieMiddleware handler = csrfSetCookieMiddleware handler (def { setCookieName = defaultCsrfCookieName }) +defaultCsrfSetCookieMiddleware handler = setCsrfCookie >> handler -- | Takes a 'SetCookie' and overrides its value with a CSRF token, then sets the cookie. See 'setCsrfCookieWithCookie'. -- -- For details, see the "AJAX CSRF protection" section of "Yesod.Core.Handler". -- +-- Make sure to set the 'setCookiePath' to the root path of your application, otherwise you'll generate a new CSRF token for every path of your app. If your app is run from from e.g. www.example.com\/app1, use @app1@. The vast majority of sites will just use @/@. +-- -- Since 1.4.14 csrfSetCookieMiddleware :: Yesod site => HandlerT site IO res -> SetCookie -> HandlerT site IO res csrfSetCookieMiddleware handler cookie = setCsrfCookieWithCookie cookie >> handler diff --git a/yesod-core/Yesod/Core/Handler.hs b/yesod-core/Yesod/Core/Handler.hs index c76dc965..32526b74 100644 --- a/yesod-core/Yesod/Core/Handler.hs +++ b/yesod-core/Yesod/Core/Handler.hs @@ -1358,7 +1358,7 @@ stripHandlerT (HandlerT f) getSub toMaster newRoute = HandlerT $ \hd -> do -- -- The form-based approach has the advantage of working for users with Javascript disabled, while adding the token to the headers with Javascript allows things like submitting JSON or binary data in AJAX requests. Yesod supports checking for a CSRF token in either the POST parameters of the form ('checkCsrfParamNamed'), the headers ('checkCsrfHeaderNamed'), or both options ('checkCsrfHeaderOrParam'). -- --- The easiest way to check both sources is to add the 'defaultCsrfMiddleware' to your Yesod Middleware. +-- The easiest way to check both sources is to add the 'Yesod.Core.defaultCsrfMiddleware' to your Yesod Middleware. -- | The default cookie name for the CSRF token ("XSRF-TOKEN"). -- @@ -1368,12 +1368,16 @@ defaultCsrfCookieName = "XSRF-TOKEN" -- | Sets a cookie with a CSRF token, using 'defaultCsrfCookieName' for the cookie name. -- +-- The cookie's path is set to @/@, making it valid for your whole website. +-- -- Since 1.4.14 setCsrfCookie :: MonadHandler m => m () -setCsrfCookie = setCsrfCookieWithCookie def { setCookieName = defaultCsrfCookieName } +setCsrfCookie = setCsrfCookieWithCookie def { setCookieName = defaultCsrfCookieName, setCookiePath = Just "/" } -- | Takes a 'SetCookie' and overrides its value with a CSRF token, then sets the cookie. -- +-- Make sure to set the 'setCookiePath' to the root path of your application, otherwise you'll generate a new CSRF token for every path of your app. If your app is run from from e.g. www.example.com\/app1, use @app1@. The vast majority of sites will just use @/@. +-- -- Since 1.4.14 setCsrfCookieWithCookie :: MonadHandler m => SetCookie -> m () setCsrfCookieWithCookie cookie = do diff --git a/yesod-core/test/YesodCoreTest/Csrf.hs b/yesod-core/test/YesodCoreTest/Csrf.hs index 8121b6da..619a53ac 100644 --- a/yesod-core/test/YesodCoreTest/Csrf.hs +++ b/yesod-core/test/YesodCoreTest/Csrf.hs @@ -45,6 +45,12 @@ csrfSpec = describe "A Yesod application with the defaultCsrfMiddleware" $ do assertStatus 200 res assertClientCookieExists "Should have an XSRF-TOKEN cookie" defaultCsrfCookieName + it "uses / as the path of the cookie" $ runner $ do -- https://github.com/yesodweb/yesod/issues/1247 + res <- request defaultRequest + assertStatus 200 res + cookiePath <- fmap setCookiePath requireCsrfCookie + liftIO $ cookiePath `shouldBe` Just "/" + it "200s write requests with the correct CSRF header, but no param" $ runner $ do getRes <- request defaultRequest assertStatus 200 getRes