Performance: Linear response time degradation when processing large numbers of validation errors

[judah-w]

A DoS vulnerability exists in how gqlgen processes queries containing a large number of invalid fields (and presumably other validation errors). When a query contains N instances of an invalid field, the engine attempts to validate and generate an error message for every single instance, leading to (O(N)) growth in processing time before a response is returned.

My suggestion is that there should be an option to abort the walk operation in the validation step once an error has been encountered.

Example:

query DoSQuery {
  user {
    # If repeated many times (e.g. 1000), CPU usage spikes
    nonExistentField 
    nonExistentField
    ...
  }
}

Proposed Solution:

PR with code changes

• Modify walk.go

type Events struct {
	// ... existing fields

	// StopOnFirstError indicates whether to stop traversal on the first error.
	StopOnFirstError bool

	// Stopped indicates traversal should stop early. This is set by validators
	// that wish to abort walking once an error has been encountered.
	Stopped bool
}

func (w *Walker) walkOperation(operation *ast.OperationDefinition) {
	// Add this logic to all walk methods
	if w.Observers != nil && w.Observers.Stopped {
		return
	}

	// ... existing code

• Modify validator.go

func ValidateWithRules(schema *Schema, doc *QueryDocument, rules *validatorrules.Rules) gqlerror.List {
	return ValidateWithRulesAndStopOnFirstError(schema, doc, rules, false)
}

func ValidateWithRulesAndStopOnFirstError(schema *Schema, doc *QueryDocument, rules *validatorrules.Rules, stopOnFirstError bool) gqlerror.List {

	// ... existing code from ValidateWithRules

	for _, currentRule := range currentRules {
		currentRule.RuleFunc(observers, func(options ...ErrorOption) {
			err := &gqlerror.Error{
				Rule: currentRule.Name,
			}
			for _, o := range options {
				o(err)
			}
			errs = append(errs, err)

			// Set stop flag if we encounter an error
			if observers.StopOnFirstError {
				observers.Stopped = true
			}
		})
	}

[StevenACoffman]

Thanks for identifying this! Rather than a boolean, I think it will be more generally useful as an integer for the maximum number of errors, where 0 is unlimited, 1 would do as this does, but you could specify some reasonable number like 10, which would cover 90% of real problems with more useful feedback, but still prevent the DoS behavior.

[judah-w]

Hi @StevenACoffman, thanks for the quick response and the suggestion! I've updated my PR to include this feature, as well as some unit tests.