Skip to content

Commit

Permalink
Give better error messages when CSRF validation fails
Browse files Browse the repository at this point in the history
* This is important because historically these errors have tripped people up
* Making security as easy as possible is important so that it doesn't just get turned off
* Giving clear directions about where to get the CSRF token (a cookie) and where to send it (a header/param) is especially helpful to frontend developers not necessarily familiar with the backend codebase
  • Loading branch information
MaxGabriel committed Nov 17, 2017
1 parent d848a71 commit 1ad4dc3
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 15 deletions.
4 changes: 4 additions & 0 deletions yesod-core/ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.4.37.1

* Improve error messages for the CSRF checking functions [#1455](https://github.com/yesodweb/yesod/issues/1455)

## 1.4.37

* Add `setWeakEtag` function in Yesod.Core.Handler module.
Expand Down
52 changes: 37 additions & 15 deletions yesod-core/Yesod/Core/Handler.hs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ import Yesod.Core.Types
import Yesod.Routes.Class (Route)
import Blaze.ByteString.Builder (Builder)
import Safe (headMay)
import Data.CaseInsensitive (CI)
import Data.CaseInsensitive (CI, original)
import qualified Data.Conduit.List as CL
import Control.Monad.Trans.Resource (MonadResource, InternalState, runResourceT, withInternalState, getInternalState, liftResourceT, resourceForkIO)
import qualified System.PosixCompat.Files as PC
Expand Down Expand Up @@ -1501,18 +1501,22 @@ defaultCsrfHeaderName = "X-XSRF-TOKEN"
-- @since 1.4.14
checkCsrfHeaderNamed :: MonadHandler m => CI S8.ByteString -> m ()
checkCsrfHeaderNamed headerName = do
valid <- hasValidCsrfHeaderNamed headerName
unless valid (permissionDenied csrfErrorMessage)
(valid, mHeader) <- hasValidCsrfHeaderNamed' headerName
unless valid (permissionDenied $ csrfErrorMessage [CSRFHeader (decodeUtf8 $ original headerName) mHeader])

-- | Takes a header name to lookup a CSRF token, and returns whether the value matches the token stored in the session.
--
-- @since 1.4.14
hasValidCsrfHeaderNamed :: MonadHandler m => CI S8.ByteString -> m Bool
hasValidCsrfHeaderNamed headerName = do
hasValidCsrfHeaderNamed headerName = fst <$> hasValidCsrfHeaderNamed' headerName

-- | Like 'hasValidCsrfHeaderNamed', but also returns the header value to be used in error messages.
hasValidCsrfHeaderNamed' :: MonadHandler m => CI S8.ByteString -> m (Bool, Maybe Text)
hasValidCsrfHeaderNamed' headerName = do
mCsrfToken <- reqToken <$> getRequest
mXsrfHeader <- lookupHeader headerName

return $ validCsrf mCsrfToken mXsrfHeader
return $ (validCsrf mCsrfToken mXsrfHeader, decodeUtf8 <$> mXsrfHeader)

-- CSRF Parameter checking

Expand All @@ -1528,18 +1532,22 @@ defaultCsrfParamName = "_token"
-- @since 1.4.14
checkCsrfParamNamed :: MonadHandler m => Text -> m ()
checkCsrfParamNamed paramName = do
valid <- hasValidCsrfParamNamed paramName
unless valid (permissionDenied csrfErrorMessage)
(valid, mParam) <- hasValidCsrfParamNamed' paramName
unless valid (permissionDenied $ csrfErrorMessage [CSRFParam paramName mParam])

-- | Takes a POST parameter name to lookup a CSRF token, and returns whether the value matches the token stored in the session.
--
-- @since 1.4.14
hasValidCsrfParamNamed :: MonadHandler m => Text -> m Bool
hasValidCsrfParamNamed paramName = do
hasValidCsrfParamNamed paramName = fst <$> hasValidCsrfParamNamed' paramName

-- | Like 'hasValidCsrfParamNamed', but also returns the param value to be used in error messages.
hasValidCsrfParamNamed' :: MonadHandler m => Text -> m (Bool, Maybe Text)
hasValidCsrfParamNamed' paramName = do
mCsrfToken <- reqToken <$> getRequest
mCsrfParam <- lookupPostParam paramName

return $ validCsrf mCsrfToken (encodeUtf8 <$> mCsrfParam)
return $ (validCsrf mCsrfToken (encodeUtf8 <$> mCsrfParam), mCsrfParam)

-- | Checks that a valid CSRF token is present in either the request headers or POST parameters.
-- If the value doesn't match the token stored in the session, this function throws a 'PermissionDenied' error.
Expand All @@ -1550,17 +1558,31 @@ checkCsrfHeaderOrParam :: (MonadHandler m, MonadLogger m)
-> Text -- ^ The POST parameter name to lookup the CSRF token
-> m ()
checkCsrfHeaderOrParam headerName paramName = do
validHeader <- hasValidCsrfHeaderNamed headerName
validParam <- hasValidCsrfParamNamed paramName
(validHeader, mHeader) <- hasValidCsrfHeaderNamed' headerName
(validParam, mParam) <- hasValidCsrfParamNamed' paramName
unless (validHeader || validParam) $ do
$logWarnS "yesod-core" csrfErrorMessage
permissionDenied csrfErrorMessage
let errorMessage = csrfErrorMessage $ [CSRFHeader (decodeUtf8 $ original headerName) mHeader, CSRFParam paramName mParam]
$logWarnS "yesod-core" errorMessage
permissionDenied errorMessage

validCsrf :: Maybe Text -> Maybe S.ByteString -> Bool
-- It's important to use constant-time comparison (constEqBytes) in order to avoid timing attacks.
validCsrf (Just token) (Just param) = encodeUtf8 token `constEqBytes` param
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. 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."
data CSRFExpectation = CSRFHeader Text (Maybe Text) -- Key/Value
| CSRFParam Text (Maybe Text) -- Key/Value

csrfErrorMessage :: [CSRFExpectation]
-> Text -- ^ Error message
csrfErrorMessage expectedLocations = "A valid CSRF token wasn't present. 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.\nTip: By default, the CSRF value is in a cookie named " `mappend` (decodeUtf8 defaultCsrfCookieName) `mappend` ".\nTip: The server is looking for the token in the following locations:\n" `mappend` T.intercalate "\n" (map csrfLocation expectedLocations)

where csrfLocation expected = case expected of
CSRFHeader k v -> T.concat ["- An HTTP header named ", k, " ", (formatValue v)]
CSRFParam k v -> T.concat ["- A POST parameter named ", k, " ", (formatValue v)]

formatValue :: Maybe Text -> Text
formatValue maybeText = case maybeText of
Nothing -> "(which is not currently set)"
Just t -> T.concat ["(which has the current, incorrect value: '", t, "')"]

0 comments on commit 1ad4dc3

Please sign in to comment.