From 28fbaae268b8fb12dd9467bcb11c739b961d26be Mon Sep 17 00:00:00 2001 From: Maximilian Tagher Date: Mon, 28 Mar 2016 12:21:50 -0700 Subject: [PATCH 1/2] Log a warning when a CSRF error occurs * Closes #1192 --- yesod-core/Yesod/Core/Handler.hs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/yesod-core/Yesod/Core/Handler.hs b/yesod-core/Yesod/Core/Handler.hs index 6dc1dd3c..f8baa37d 100644 --- a/yesod-core/Yesod/Core/Handler.hs +++ b/yesod-core/Yesod/Core/Handler.hs @@ -252,6 +252,7 @@ import qualified Yesod.Core.TypeCache as Cache import qualified Data.Word8 as W8 import qualified Data.Foldable as Fold import Data.Default +import Control.Monad.Logger (MonadLogger, logWarnS) get :: MonadHandler m => m GHState get = liftHandlerT $ HandlerT $ I.readIORef . handlerState @@ -1439,14 +1440,16 @@ hasValidCsrfParamNamed paramName = do -- If the value doesn't match the token stored in the session, this function throws a 'PermissionDenied' error. -- -- Since 1.4.14 -checkCsrfHeaderOrParam :: MonadHandler m +checkCsrfHeaderOrParam :: (MonadHandler m, MonadLogger m) => CI S8.ByteString -- ^ The header name to lookup the CSRF token -> Text -- ^ The POST parameter name to lookup the CSRF token -> m () checkCsrfHeaderOrParam headerName paramName = do validHeader <- hasValidCsrfHeaderNamed headerName validParam <- hasValidCsrfParamNamed paramName - unless (validHeader || validParam) (permissionDenied csrfErrorMessage) + unless (validHeader || validParam) $ do + $logWarnS "yesod-core" csrfErrorMessage + permissionDenied csrfErrorMessage validCsrf :: Maybe Text -> Maybe S.ByteString -> Bool -- It's important to use constant-time comparison (constEqBytes) in order to avoid timing attacks. From 5a5cfd6c7a6284b6912dc3247ced6d8a7202efcc Mon Sep 17 00:00:00 2001 From: Maximilian Tagher Date: Mon, 28 Mar 2016 23:33:32 -0700 Subject: [PATCH 2/2] Bump version for CSRF logging changes, and improve error message. --- yesod-core/ChangeLog.md | 4 ++++ yesod-core/Yesod/Core/Handler.hs | 2 +- yesod-core/yesod-core.cabal | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/yesod-core/ChangeLog.md b/yesod-core/ChangeLog.md index b8aec9c8..4bdfe7bd 100644 --- a/yesod-core/ChangeLog.md +++ b/yesod-core/ChangeLog.md @@ -1,3 +1,7 @@ +## 1.4.20.1 + +* Log a warning when a CSRF error occurs [#1200](https://github.com/yesodweb/yesod/pull/1200) + ## 1.4.20 * `addMessage`, `addMessageI`, and `getMessages` functions diff --git a/yesod-core/Yesod/Core/Handler.hs b/yesod-core/Yesod/Core/Handler.hs index f8baa37d..a0230e18 100644 --- a/yesod-core/Yesod/Core/Handler.hs +++ b/yesod-core/Yesod/Core/Handler.hs @@ -1458,4 +1458,4 @@ validCsrf Nothing _param = True validCsrf (Just _token) Nothing = False csrfErrorMessage :: Text -csrfErrorMessage = "A valid CSRF token wasn't present in HTTP headers or POST parameters. Check the Yesod.Core.Handler docs of the yesod-core package for details on CSRF protection." +csrfErrorMessage = "A valid CSRF token wasn't present in HTTP headers or POST parameters. Because the request could have been forged, it's been rejected altogether. Check the Yesod.Core.Handler docs of the yesod-core package for details on CSRF protection." diff --git a/yesod-core/yesod-core.cabal b/yesod-core/yesod-core.cabal index efd9aa03..1ed8b27b 100644 --- a/yesod-core/yesod-core.cabal +++ b/yesod-core/yesod-core.cabal @@ -1,5 +1,5 @@ name: yesod-core -version: 1.4.20 +version: 1.4.20.1 license: MIT license-file: LICENSE author: Michael Snoyman