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

Add a security section warning about CSRF #64

Closed
wants to merge 3 commits into from

Conversation

glasser
Copy link

@glasser glasser commented May 25, 2022

Adding a multipart/form-data parser to a server that previously only parsed application/json POSTs increases its vulnerability to CSRF attacks, because multipart/form-data is special-cased by browser security rules. Anyone implementing this protocol should be aware of this.

Adding a `multipart/form-data` parser to a server that previously only parsed `application/json` POSTs increases its vulnerability to CSRF attacks, because `multipart/form-data` is special-cased by browser security rules. Anyone implementing this protocol should be aware of this.
@glasser
Copy link
Author

glasser commented May 25, 2022

I think this text also belongs in other libraries such as graphql-upload but let's start by being happy with it in one place.

Copy link

@spawnia spawnia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good writeup. It makes sense, is precise, concise, and helpful.

readme.md Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
glasser and others added 2 commits April 12, 2023 10:07
Co-authored-by: Benedikt Franke <benedikt@franke.tech>
@jaydenseric
Copy link
Owner

Thank you @glasser for the concern and effort you have put into raising awareness about GraphQL server authentication and security mechanisms that can be used to thwart CSRF attacks via requests with a Content-Type of multipart/form-data.

In 9322085 I added a “Security” section that clarifies that GraphQL server authentication and security mechanisms are beyond the scope of this specification, which only covers a multipart form field structure for GraphQL requests.

I did however add this note:

Note that a GraphQL multipart request has the Content-Type multipart/form-data; if a browser making such a request determines it meets the criteria for a “simple request” as defined in the Fetch specification for the Cross-Origin Resource Sharing (CORS) protocol, it won’t cause a CORS preflight request. GraphQL server authentication and security mechanisms must consider this to prevent Cross-Site Request Forgery (CSRF) attacks.

I'm comfortable highlighting an unintuitive browser behavior as a favour to the community to increase awareness (even if it is out of scope for this spec), but I'm not a security subject matter expert and as such am not comfortable about providing specific advice to people about whether or not their particular server setup is affected, or attempting to prescribe particular authentication and security mechanisms tailored to different kinds of setups. Even if I was an expert, getting the wording just right and explaining everything in adequate detail is a cognitive burden I simply don't have the time to produce and maintain right now.

There certainly is room out there for more detailed articles and blog posts on the topic, but note that multipart requests are used for all sorts of APIs out there, not just GraphQL file uploads, and a lot of that advice could be general to any API server that has an endpoint accepting multipart requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants