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

GraphQL nesting security limit #1058

Closed
1 task done
f2net opened this issue Apr 28, 2018 · 6 comments
Closed
1 task done

GraphQL nesting security limit #1058

f2net opened this issue Apr 28, 2018 · 6 comments
Assignees
Labels
issue: feature request Issue suggesting a new feature severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve

Comments

@f2net
Copy link

f2net commented Apr 28, 2018

What is the expected behavior?

I think there should be a limit (set by admin interface) to the nesting depth of a GraphQL query.
Otherwise launching queries like the one below could overload the server:

beers { style { beers { style { beers { style { beers { Name }}}}}}}

What do you think about it?


  • I'm sure that this issue hasn't already been referenced
@lauriejim lauriejim added the issue: feature request Issue suggesting a new feature label Apr 28, 2018
@Aurelsicoko
Copy link
Member

You can already use the limit parameter for avoiding this.

beers (limit: 20) { 
  style { 
    beers (limit: 20) { 
      style { 
        beers (limit : 20) {
          name
        }
      }
    }
  }
}  

Also, I'm not sure it's a great thing to put a default limit because how do you handle the case when you want to display all the articles of a user?

users (limit: 20) {
  articles { // I don't want any limit on this relation
    title
    description
  }
}

@f2net
Copy link
Author

f2net commented May 2, 2018

Aurelien, I didn't mean a limit on the number of records, but a limit on the depth of the nesting.
If it is possible to nest unlimited sub-queries, the api is subject to performance attacks causing heavy loads on the server.
@Aurelsicoko @pierreburgy

See this article by Max Stoiber:
https://dev-blog.apollodata.com/securing-your-graphql-api-from-malicious-queries-16130a324a6b

@Aurelsicoko
Copy link
Member

@f2net Oh yes, I already read it! You're right we have to limit the depth of the nesting.

@lauriejim lauriejim added the severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve label May 7, 2018
@dsheyp
Copy link

dsheyp commented May 9, 2018

hi

I'm doing this query - which will not work as expected:
{ sections{title index slides{ title index videos{ title videoid } bgimages { title image { url } } } } }

image will always be image: null

is this related to a limited depth of nesting or is it something else?

when i directly request http://localhost:1337/bgimage then the image property is not null

if this query is used "standalone" it also works: {bgimages { title image { url } } }

thx

@Aurelsicoko
Copy link
Member

@dsheyp You're not the first one to report me this issue. Can you open another issue?

@Aurelsicoko
Copy link
Member

I added the amount limiting and depth limiting feature. I think 7 levels of depths is enough. What do you think about that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: feature request Issue suggesting a new feature severity: low If the issue only affects a very niche base of users and an easily implemented workaround can solve
Projects
None yet
Development

No branches or pull requests

3 participants