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

Deprecate insecure JSON body functions #1576

Conversation

Projects
None yet
2 participants
@eborden
Copy link
Contributor

eborden commented Jan 23, 2019

parseJsonBody and requireJsonBody do not require a mime type when
parsing JSON content. This leaves them open to CSRF. They are now
deprecated and insecure versions are added in their place. Consumers
are now given a proper choice between secure and insecure functions.

There is a potential attack vector in that the browser does not trigger
CORS requests for "simple requests", which includes POST requests that
are form or text content-types. An attacker can craft a form whose body
is valid JSON, and when a user visits attacker.com and submits that
form, it can be submitted to bank.com and bypass CORS.

Checking the content-type is application/json prevents this, because if
the content-type was set to application/json, then the browser would
send a CORS request—a preflight OPTIONS request to the server asking if
the current domain (and some other values) are whitelisted to send
requests to that server. If the server doesn't say attacker.com is
whitelisted, the browser will not send the real request to the server.

Before submitting your PR, check that you've:

After submitting your PR:

  • Update the Changelog.md file with a link to your PR
  • Check that CI passes (or if it fails, for reasons unrelated to your change, like CI timeouts)
@eborden

This comment has been minimized.

Copy link
Contributor Author

eborden commented Jan 23, 2019

This addresses #1575

@MaxGabriel, I'm not sure what yesod's versioning policy is. I would consider a deprecation to be a major bump.

Show resolved Hide resolved yesod-core/Yesod/Core/Json.hs Outdated
@snoyberg
Copy link
Member

snoyberg left a comment

Code looks good, just a request about the stylish changes.

Show resolved Hide resolved yesod-core/Yesod/Core/Json.hs Outdated
Deprecate insecure JSON body functions
`parseJsonBody` and `requireJsonBody` do not require a mime type when
parsing `JSON` content. This leaves them open to CSRF. They are now
deprecated and `insecure` versions are added in their place. Consumers
are now given a proper choice between secure and insecure functions.

There is a potential attack vector in that the browser does not trigger
CORS requests for "simple requests", which includes POST requests that
are form or text content-types. An attacker can craft a form whose body
is valid JSON, and when a user visits attacker.com and submits that
form, it can be submitted to bank.com and bypass CORS.

Checking the content-type is application/json prevents this, because if
the content-type was set to application/json, then the browser would
send a CORS request—a preflight OPTIONS request to the server asking if
the current domain (and some other values) are whitelisted to send
requests to that server. If the server doesn't say attacker.com is
whitelisted, the browser will not send the real request to the server.

@eborden eborden force-pushed the eborden:eborden/deprecate-insecure-json-body-functions branch from 9f6fa25 to b50ca99 Jan 24, 2019

@snoyberg
Copy link
Member

snoyberg left a comment

Thanks!

Show resolved Hide resolved yesod-core/Yesod/Core/Json.hs
Add minor version bump to 1.6.11
JSON parsing function deprecations warrant a minor version bump.
@snoyberg
Copy link
Member

snoyberg left a comment

Thanks!

@snoyberg snoyberg merged commit 9ccdc38 into yesodweb:master Jan 30, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eborden eborden deleted the eborden:eborden/deprecate-insecure-json-body-functions branch Jan 30, 2019

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