Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

yesod-auth: Fix CSRF security vulnerability in registerHelper function #1302

Merged
merged 3 commits into from Nov 22, 2016

Conversation

@psibi
Copy link
Member

commented Nov 19, 2016

Return a 403 status code if the csrf tokens aren't matched. This currently
affects two endpoints: During registration and during password reset
forms.

This curl request demonstrates how this can be exploited to register new
email:

curl -i --header "Accept: application/json" --request POST -F
"email=sibi@psibi.in" http://localhost:3005/auth/page/email/register

With the patch applied, it will respond with this:

{"message":"Permission Denied. 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."}

Return a 403 status code if the csrf tokens are matched. This currently
affects two endpoints: During registration and during password reset
forms.

This curl request demonstrates how this can be exploited to register new
email:

curl -i --header "Accept: application/json" --request POST -F
"email=sibi@psibi.in" http://localhost:3005/auth/page/email/register

With the patch applied, it will respond with this:

{"message":"Permission Denied. 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."}
@psibi psibi added the in progress label Nov 19, 2016
@@ -709,6 +713,9 @@ setLoginLinkKey aid = do
now <- liftIO getCurrentTime
setSession loginLinkKey $ TS.pack $ show (toPathPiece aid, now)

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."

This comment has been minimized.

Copy link
@MaxGabriel

MaxGabriel Nov 20, 2016

Member

It's a little weird because it's not an API the user would use, but I think exporting this error message from Yesod-core is preferable to pasting it here

| otherwise -> Left Msg.InvalidEmailAddress

case eidentifier of
csrfToken <- lookupPostParam "_token"

This comment has been minimized.

Copy link
@MaxGabriel

MaxGabriel Nov 20, 2016

Member

The error message used below says the the CSRF token can come from the headers or in the post parameters, but afaict the code only checks the post parameters. You could use checkCsrfHeaderOrParam to fix this. Also this would obviate the need to paste the CSRF error message from below

This comment has been minimized.

Copy link
@psibi

psibi Nov 20, 2016

Author Member

Thanks for the feedback. Indeed checkCsrfHeaderOrParam is the right way to do. I have pushed the code with fixes.

psibi added 2 commits Nov 20, 2016
@MaxGabriel

This comment has been minimized.

Copy link
Member

commented Nov 20, 2016

This LGTM, will merge Tuesday morning absent any other comments.

@MaxGabriel MaxGabriel merged commit 54cc420 into yesodweb:master Nov 22, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@snoyberg snoyberg removed the in progress label Nov 22, 2016
@MaxGabriel

This comment has been minimized.

Copy link
Member

commented Nov 22, 2016

Nice fix @psibi! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.