possible denial of service attack due to unlimited errors CVE-2023-49559

[PookieTek]

By running the following query against our gqlgen based servers:

query {
  __typename @a @b @c @d @e ... # imagine 100k+ more of these
}

We can reliably get them OOM killed in our k8s cluster.

The query is taken from https://bessey.dev/blog/2024/05/24/why-im-over-graphql/ and the suggested solution is to limit the number of errors handled.

We could improve the situation a bit by limiting the actual number of errors returned in the response. However the issue still persists because we can only limit the list before returning the response - not preventing it from growing during parsing.
If I send enough directive, any server will crash. We should expose a limit to the amount of tokens without breaking existing behavior (unlimited tokens).

versions

• go run github.com/99designs/gqlgen version? v0.17.40
• go version? go1.22.3

[xaviergodart]

I came here with the same question. It seems that it should be the responsibility of gqlparser and I just saw there's an ongoing discussion here: vektah/gqlparser#303

[StevenACoffman]

I'm open to a PR here in gqlgen that would add a configurable token limit, but leave the default as 0 (unlimited).

it looks like over in Java what they actually did was limit the number of tokens they parse. (See graphql-java/graphql-java#2892. They also have similar limits for comments/whitespace.) That seems a lot simpler and more general. Or, it may even suffice to simply limit the request size. That doesn't need to be in the parser at all -- many HTTP servers already do so or callers could do so via middleware -- so it could just be something to document, or for server libraries to have a simple default which users who want something more complex (e.g. different limits for different users) can replace with such middleware. Of course, public GraphQL servers may also wish to limit query complexity in other ways, as they can do via gqlgen's complexity APIs.

[xaviergodart]

I'm taking a quick look at this and it doesn't seem that gqlparser exposes a way to make it configurable, unless I'm missing something.

I see that the method SetMaxTokenLimit has been added to the parser struct here, but the struct is private and gqlgen never manipulate parser directly. It uses ParseQuery (here) that handles the parser under the hood.

Not sure what would be the best approach to expose this though. A solution would be to add functional options to ParseQuery on gqlparser, something like:

func ParseQuery(source *Source, options ...parserOption) { ... }

func WithTokenLimit(limit int) parserOption { ... }

// you would call ParseQuery like this
doc, err := parser.ParseQuery(&ast.Source{Input: query}, WithTokenLimit(2000))

[StevenACoffman]

🤦 Ugh, you are entirely correct @xaviergodart . See github.com/vektah/gqlparser#304 and https://github.com/vektah/gqlparser/releases/tag/v2.5.15

I don't want to break backwards compatibility for any of the existing consumers of gqlparser library, so people can opt-in to the new behavior using ParseQueryWithLimit. If we started to develop a profusion of parser options, we could provide a ParseQueryWithParser, but this library has historically had a pretty slow rate of change.

[StevenACoffman]

Anyway, @xaviergodart or @PookieTek please submit a PR here that adds a configurable token limit to gqlgen. The default should be 0 (unlimited).

[StevenACoffman]

Fixed in #3136