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

Disable graphql-upload integration when it is not used #6476

Merged
merged 1 commit into from
May 25, 2022

Conversation

glasser
Copy link
Member

@glasser glasser commented May 25, 2022

By default, we run the graphql-upload middleware on all requests. This
middleware is vulnerable to mutation CSRF attacks because it parses POST
requests with content-type: multipart/form-data, which can happen in a
non-preflighted browser request. (Without graphql-upload, Apollo Server
won't process any mutations in non-preflighted requests, because
mutations must be in POST requests and normally that requires
content-type: application/json which must be preflighted.)

In order to safely use graphql-upload, you should upgrade to Apollo
Server v3.7 and use its new CSRF prevention feature. Because Apollo
Server 2 is not under active development we do not intend to backport
the full CSRF prevention feature to AS2.

However, we at least want to protect the users of Apollo Server 2 who
don't actually need graphql-upload to be enabled (which is probably
most of them). This PR changes the default behavior of Apollo Server 2
when no uploads parameter is passed. Instead of always executing the
graphql-upload middleware in this case, we only execute it if the
Upload scalar (which may be added automatically to the schema by AS
itself or may be provided by the user) is referenced somewhere in the
schema other than its own definition. This should be roughly
backwards-compatible; it only breaks the ability to use a
graphql-upload-based client with Apollo Servers that don't accept
uploads.

We also print a warning when uploads are enabled encouraging upgrades.

Part of GHSA-2p3c-p3qw-69r4

@netlify
Copy link

netlify bot commented May 25, 2022

Deploy Preview for apollo-server-docs ready!

Name Link
🔨 Latest commit 710ac11
🔍 Latest deploy log https://app.netlify.com/sites/apollo-server-docs/deploys/628e5ce6107a5800085f3a36
😎 Deploy Preview https://deploy-preview-6476--apollo-server-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

By default, we run the graphql-upload middleware on all requests. This
middleware is vulnerable to mutation CSRF attacks because it parses POST
requests with `content-type: multipart/form-data`, which can happen in a
non-preflighted browser request. (Without graphql-upload, Apollo Server
won't process any mutations in non-preflighted requests, because
mutations must be in POST requests and normally that requires
`content-type: application/json` which must be preflighted.)

In order to safely use graphql-upload, you should upgrade to Apollo
Server v3.7 and use its new CSRF prevention feature. Because Apollo
Server 2 is not under active development we do not intend to backport
the full CSRF prevention feature to AS2.

However, we at least want to protect the users of Apollo Server 2 who
*don't* actually need graphql-upload to be enabled (which is probably
most of them). This PR changes the default behavior of Apollo Server 2
when no `uploads` parameter is passed. Instead of always executing the
graphql-upload middleware in this case, we only execute it if the
`Upload` scalar (which may be added automatically to the schema by AS
itself or may be provided by the user) is referenced somewhere in the
schema other than its own definition. This should be roughly
backwards-compatible; it only breaks the ability to use a
`graphql-upload`-based client with Apollo Servers that don't accept
uploads.

We also print a warning when uploads are enabled encouraging upgrades.
@glasser glasser force-pushed the glasser/as2-csrf branch from 4c5773b to 710ac11 Compare May 25, 2022 16:44
@glasser glasser merged commit 82d4498 into version-2 May 25, 2022
@glasser glasser deleted the glasser/as2-csrf branch May 25, 2022 16:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant