Commit a6adb336 authored by GitLab Release Tools Bot's avatar GitLab Release Tools Bot

Merge branch 'security-64519-circular-graphql-queries-12-4' into '12-4-stable'

Nested GraphQL query with circular relationship can cause Denial of Service

See merge request gitlab/gitlabhq!3492
parents bbe85167 77f685a8
...@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema ...@@ -18,15 +18,15 @@ class GitlabSchema < GraphQL::Schema
use Gitlab::Graphql::GenericTracing use Gitlab::Graphql::GenericTracing
query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new query_analyzer Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer.new
query_analyzer Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer.new
query(Types::QueryType)
default_max_page_size 100
max_complexity DEFAULT_MAX_COMPLEXITY max_complexity DEFAULT_MAX_COMPLEXITY
max_depth DEFAULT_MAX_DEPTH max_depth DEFAULT_MAX_DEPTH
mutation(Types::MutationType) query Types::QueryType
mutation Types::MutationType
default_max_page_size 100
class << self class << self
def multiplex(queries, **kwargs) def multiplex(queries, **kwargs)
......
---
title: Analyze incoming GraphQL queries and check for recursion
merge_request:
author:
type: security
# frozen_string_literal: true
# Recursive queries, with relatively low effort, can quickly spiral out of control exponentially
# and may not be picked up by depth and complexity alone.
module Gitlab
module Graphql
module QueryAnalyzers
class RecursionAnalyzer
IGNORED_FIELDS = %w(node edges ofType).freeze
RECURSION_THRESHOLD = 2
def initial_value(query)
{
recurring_fields: {}
}
end
def call(memo, visit_type, irep_node)
return memo if skip_node?(irep_node)
node_name = irep_node.ast_node.name
times_encountered = memo[node_name] || 0
if visit_type == :enter
times_encountered += 1
memo[:recurring_fields][node_name] = times_encountered if recursion_too_deep?(node_name, times_encountered)
else
times_encountered -= 1
end
memo[node_name] = times_encountered
memo
end
def final_value(memo)
recurring_fields = memo[:recurring_fields]
recurring_fields = recurring_fields.select { |k, v| recursion_too_deep?(k, v) }
if recurring_fields.any?
GraphQL::AnalysisError.new("Recursive query - too many of fields '#{recurring_fields}' detected in single branch of the query")
end
end
private
def recursion_too_deep?(node_name, times_encountered)
return if IGNORED_FIELDS.include?(node_name)
times_encountered > recursion_threshold
end
def skip_node?(irep_node)
ast_node = irep_node.ast_node
!ast_node.is_a?(GraphQL::Language::Nodes::Field) || ast_node.selections.empty?
end
def recursion_threshold
RECURSION_THRESHOLD
end
end
end
end
end
query allSchemaTypes {
__schema {
types {
fields {
type{
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
type {
fields {
name
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
\ No newline at end of file
{
project(fullPath: "gitlab-org/gitlab-ce") {
group {
projects {
edges {
node {
group {
projects {
edges {
node {
group {
projects {
edges {
node {
group {
projects {
edges {
node {
group {
projects {
edges {
node {
group {
description
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
}
query allSchemaTypes {
__schema {
types {
fields {
type {
fields {
type {
name
}
}
}
}
}
}
}
...@@ -13,7 +13,7 @@ describe 'GitlabSchema configurations' do ...@@ -13,7 +13,7 @@ describe 'GitlabSchema configurations' do
subject subject
expect(graphql_errors.flatten.first['message']).to include('which exceeds max complexity of 1') expect_graphql_errors_to_include /which exceeds max complexity of 1/
end end
end end
end end
...@@ -21,12 +21,11 @@ describe 'GitlabSchema configurations' do ...@@ -21,12 +21,11 @@ describe 'GitlabSchema configurations' do
describe '#max_depth' do describe '#max_depth' do
context 'when query depth is too high' do context 'when query depth is too high' do
it 'shows error' do it 'shows error' do
errors = { "message" => "Query has depth of 2, which exceeds max depth of 1" }
allow(GitlabSchema).to receive(:max_query_depth).and_return 1 allow(GitlabSchema).to receive(:max_query_depth).and_return 1
subject subject
expect(graphql_errors.flatten).to include(errors) expect_graphql_errors_to_include /exceeds max depth/
end end
end end
...@@ -36,7 +35,42 @@ describe 'GitlabSchema configurations' do ...@@ -36,7 +35,42 @@ describe 'GitlabSchema configurations' do
subject subject
expect(Array.wrap(graphql_errors).compact).to be_empty expect_graphql_errors_to_be_empty
end
end
end
end
context 'depth, complexity and recursion checking' do
context 'unauthenticated recursive queries' do
context 'a not-quite-recursive-enough introspective query' do
it 'succeeds' do
query = File.read(Rails.root.join('spec/fixtures/api/graphql/small-recursive-introspection.graphql'))
post_graphql(query, current_user: nil)
expect_graphql_errors_to_be_empty
end
end
context 'a deep but simple recursive introspective query' do
it 'fails due to recursion' do
query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-introspection.graphql'))
post_graphql(query, current_user: nil)
expect_graphql_errors_to_include [/Recursive query/]
end
end
context 'a deep recursive non-introspective query' do
it 'fails due to recursion, complexity and depth' do
allow(GitlabSchema).to receive(:max_query_complexity).and_return 1
query = File.read(Rails.root.join('spec/fixtures/api/graphql/recursive-query.graphql'))
post_graphql(query, current_user: nil)
expect_graphql_errors_to_include [/Recursive query/, /exceeds max complexity/, /exceeds max depth/]
end end
end end
end end
...@@ -86,7 +120,7 @@ describe 'GitlabSchema configurations' do ...@@ -86,7 +120,7 @@ describe 'GitlabSchema configurations' do
# Expect errors for each query # Expect errors for each query
expect(graphql_errors.size).to eq(3) expect(graphql_errors.size).to eq(3)
graphql_errors.each do |single_query_errors| graphql_errors.each do |single_query_errors|
expect(single_query_errors.first['message']).to include('which exceeds max complexity of 4') expect_graphql_errors_to_include(/which exceeds max complexity of 4/)
end end
end end
end end
...@@ -103,12 +137,12 @@ describe 'GitlabSchema configurations' do ...@@ -103,12 +137,12 @@ describe 'GitlabSchema configurations' do
end end
context 'when IntrospectionQuery' do context 'when IntrospectionQuery' do
it 'is not too complex' do it 'is not too complex nor recursive' do
query = File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql')) query = File.read(Rails.root.join('spec/fixtures/api/graphql/introspection.graphql'))
post_graphql(query, current_user: nil) post_graphql(query, current_user: nil)
expect(graphql_errors).to be_nil expect_graphql_errors_to_be_empty
end end
end end
......
...@@ -129,6 +129,7 @@ module GraphqlHelpers ...@@ -129,6 +129,7 @@ module GraphqlHelpers
allow_unlimited_graphql_complexity allow_unlimited_graphql_complexity
allow_unlimited_graphql_depth allow_unlimited_graphql_depth
allow_high_graphql_recursion
type = GitlabSchema.types[class_name.to_s] type = GitlabSchema.types[class_name.to_s]
return "" unless type return "" unless type
...@@ -213,6 +214,23 @@ module GraphqlHelpers ...@@ -213,6 +214,23 @@ module GraphqlHelpers
end end
end end
def expect_graphql_errors_to_include(regexes_to_match)
raise "No errors. Was expecting to match #{regexes_to_match}" if graphql_errors.nil? || graphql_errors.empty?
error_messages = flattened_errors.collect { |error_hash| error_hash["message"] }
Array.wrap(regexes_to_match).flatten.each do |regex|
expect(error_messages).to include a_string_matching regex
end
end
def expect_graphql_errors_to_be_empty
expect(flattened_errors).to be_empty
end
def flattened_errors
Array.wrap(graphql_errors).flatten.compact
end
# Raises an error if no response is found # Raises an error if no response is found
def graphql_mutation_response(mutation_name) def graphql_mutation_response(mutation_name)
graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name)) graphql_data.fetch(GraphqlHelpers.fieldnamerize(mutation_name))
...@@ -260,6 +278,10 @@ module GraphqlHelpers ...@@ -260,6 +278,10 @@ module GraphqlHelpers
allow_any_instance_of(GitlabSchema).to receive(:max_depth).and_return nil allow_any_instance_of(GitlabSchema).to receive(:max_depth).and_return nil
allow(GitlabSchema).to receive(:max_query_depth).with(any_args).and_return nil allow(GitlabSchema).to receive(:max_query_depth).with(any_args).and_return nil
end end
def allow_high_graphql_recursion
allow_any_instance_of(Gitlab::Graphql::QueryAnalyzers::RecursionAnalyzer).to receive(:recursion_threshold).and_return 1000
end
end end
# This warms our schema, doing this as part of loading the helpers to avoid # This warms our schema, doing this as part of loading the helpers to avoid
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment