Add latest changes from gitlab-org/security/gitlab@14-5-stable-ee

7 files changed, 98 insertions, 4 deletions

diff --git a/Gemfile b/Gemfile
index 87a0cff84c1..7fa615319c4 100644
--- a/Gemfile
+++ b/Gemfile
@@ -97,7 +97,7 @@ gem 'grape-entity', '~> 0.10.0'
 gem 'rack-cors', '~> 1.0.6', require: 'rack/cors'
 
 # GraphQL API
-gem 'graphql', '~> 1.11.8'
+gem 'graphql', '~> 1.11.10'
 # NOTE: graphiql-rails v1.5+ doesn't work: https://gitlab.com/gitlab-org/gitlab/issues/31771
 # TODO: remove app/views/graphiql/rails/editors/show.html.erb when https://github.com/rmosolgo/graphiql-rails/pull/71 is released:
 # https://gitlab.com/gitlab-org/gitlab/issues/31747
diff --git a/Gemfile.lock b/Gemfile.lock
index 270ea4532f9..b54874e9d80 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -558,7 +558,7 @@ GEM
       faraday (>= 1.0)
       faraday_middleware
       graphql-client
-    graphql (1.11.8)
+    graphql (1.11.10)
     graphql-client (0.16.0)
       activesupport (>= 3.0)
       graphql (~> 1.8)
@@ -1490,7 +1490,7 @@ DEPENDENCIES
   grape_logging (~> 1.7)
   graphiql-rails (~> 1.4.10)
   graphlient (~> 0.4.0)
-  graphql (~> 1.11.8)
+  graphql (~> 1.11.10)
   graphql-docs (~> 1.6.0)
   grpc (~> 1.30.2)
   gssapi
diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb
index fde0f133e53..899fa614949 100644
--- a/app/controllers/graphql_controller.rb
+++ b/app/controllers/graphql_controller.rb
@@ -9,6 +9,9 @@ class GraphqlController < ApplicationController
   # Header can be passed by tests to disable SQL query limits.
   DISABLE_SQL_QUERY_LIMIT_HEADER = 'HTTP_X_GITLAB_DISABLE_SQL_QUERY_LIMIT'
 
+  # Max size of the query text in characters
+  MAX_QUERY_SIZE = 10_000
+
   # If a user is using their session to access GraphQL, we need to have session
   # storage, since the admin-mode check is session wide.
   # We can't enable this for anonymous users because that would cause users using
@@ -29,6 +32,7 @@ class GraphqlController < ApplicationController
   before_action :set_user_last_activity
   before_action :track_vs_code_usage
   before_action :disable_query_limiting
+  before_action :limit_query_size
 
   before_action :disallow_mutations_for_get
 
@@ -81,6 +85,16 @@ class GraphqlController < ApplicationController
     raise ::Gitlab::Graphql::Errors::ArgumentError, "Mutations are forbidden in #{request.request_method} requests"
   end
 
+  def limit_query_size
+    total_size = if multiplex?
+                   params[:_json].sum { _1[:query].size }
+                 else
+                   query.size
+                 end
+
+    raise ::Gitlab::Graphql::Errors::ArgumentError, "Query too large" if total_size > MAX_QUERY_SIZE
+  end
+
   def any_mutating_query?
     if multiplex?
       multiplex_queries.any? { |q| mutation?(q[:query], q[:operation_name]) }
@@ -126,7 +140,7 @@ class GraphqlController < ApplicationController
   end
 
   def query
-    params[:query]
+    params.fetch(:query, '')
   end
 
   def multiplex_queries
diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb
index e15a185a743..9b23aa60eab 100644
--- a/app/graphql/gitlab_schema.rb
+++ b/app/graphql/gitlab_schema.rb
@@ -32,6 +32,9 @@ class GitlabSchema < GraphQL::Schema
 
   default_max_page_size 100
 
+  validate_max_errors 5
+  validate_timeout 0.2.seconds
+
   lazy_resolve ::Gitlab::Graphql::Lazy, :force
 
   class << self
diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb
index 6e7bcfdaa08..f9b15c9a48e 100644
--- a/spec/controllers/graphql_controller_spec.rb
+++ b/spec/controllers/graphql_controller_spec.rb
@@ -52,6 +52,44 @@ RSpec.describe GraphqlController do
         expect(response).to have_gitlab_http_status(:ok)
       end
 
+      it 'executes a simple query with no errors' do
+        post :execute, params: { query: '{ __typename }' }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to eq({ 'data' => { '__typename' => 'Query' } })
+      end
+
+      it 'executes a simple multiplexed query with no errors' do
+        multiplex = [{ query: '{ __typename }' }] * 2
+
+        post :execute, params: { _json: multiplex }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to eq([
+          { 'data' => { '__typename' => 'Query' } },
+          { 'data' => { '__typename' => 'Query' } }
+        ])
+      end
+
+      it 'sets a limit on the total query size' do
+        graphql_query = "{#{(['__typename'] * 1000).join(' ')}}"
+
+        post :execute, params: { query: graphql_query }
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        expect(json_response).to eq({ 'errors' => [{ 'message' => 'Query too large' }] })
+      end
+
+      it 'sets a limit on the total query size for multiplex queries' do
+        graphql_query = "{#{(['__typename'] * 200).join(' ')}}"
+        multiplex = [{ query: graphql_query }] * 5
+
+        post :execute, params: { _json: multiplex }
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        expect(json_response).to eq({ 'errors' => [{ 'message' => 'Query too large' }] })
+      end
+
       it 'returns forbidden when user cannot access API' do
         # User cannot access API in a couple of cases
         # * When user is internal(like ghost users)
diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb
index 3fa0dc95126..02c686af688 100644
--- a/spec/graphql/gitlab_schema_spec.rb
+++ b/spec/graphql/gitlab_schema_spec.rb
@@ -35,6 +35,10 @@ RSpec.describe GitlabSchema do
     expect(connection).to eq(Gitlab::Graphql::Pagination::ExternallyPaginatedArrayConnection)
   end
 
+  it 'sets an appropriate validation timeout' do
+    expect(described_class.validate_timeout).to be <= 0.2.seconds
+  end
+
   describe '.execute' do
     describe 'setting query `max_complexity` and `max_depth`' do
       subject(:result) { described_class.execute('query', **kwargs).query }
@@ -195,6 +199,36 @@ RSpec.describe GitlabSchema do
     end
   end
 
+  describe 'validate_max_errors' do
+    it 'reports at most 5 errors' do
+      query = <<~GQL
+        query {
+          currentUser {
+            x: id
+            x: bot
+            x: username
+            x: state
+            x: name
+
+            x: id
+            x: bot
+            x: username
+            x: state
+            x: name
+
+            badField
+            veryBadField
+            alsoNotAGoodField
+          }
+        }
+      GQL
+
+      result = described_class.execute(query)
+
+      expect(result.to_h['errors'].count).to eq 5
+    end
+  end
+
   describe '.parse_gid' do
     let_it_be(:global_id) { 'gid://gitlab/TestOne/2147483647' }
 
diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb
index ee4621deb2d..1f0c9b658dc 100644
--- a/spec/support/helpers/graphql_helpers.rb
+++ b/spec/support/helpers/graphql_helpers.rb
@@ -374,6 +374,7 @@ module GraphqlHelpers
     allow_unlimited_graphql_depth if max_depth > 1
     allow_high_graphql_recursion
     allow_high_graphql_transaction_threshold
+    allow_high_graphql_query_size
 
     type = class_name.respond_to?(:kind) ? class_name : GitlabSchema.types[class_name.to_s]
     raise "#{class_name} is not a known type in the GitlabSchema" unless type
@@ -624,6 +625,10 @@ module GraphqlHelpers
     stub_const("Gitlab::QueryLimiting::Transaction::THRESHOLD", 1000)
   end
 
+  def allow_high_graphql_query_size
+    stub_const('GraphqlController::MAX_QUERY_SIZE', 10_000_000)
+  end
+
   def node_array(data, extract_attribute = nil)
     data.map do |item|
       extract_attribute ? item['node'][extract_attribute] : item['node']