Add latest changes from gitlab-org/security/gitlab@13-10-stable-ee

app/models/merge_request.rb

@@ -1337,8 +1337,8 @@ class MergeRequest < ApplicationRecord
     has_no_commits? || branch_missing? || cannot_be_merged?
   end
 
-  def can_be_merged_by?(user)
-    access = ::Gitlab::UserAccess.new(user, container: project)
+  def can_be_merged_by?(user, skip_collaboration_check: false)
+    access = ::Gitlab::UserAccess.new(user, container: project, skip_collaboration_check: skip_collaboration_check)
     access.can_update_branch?(target_branch)
   end
 

app/models/project.rb

@@ -2704,7 +2704,7 @@ class Project < ApplicationRecord
       # Issue for N+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/49322
       Gitlab::GitalyClient.allow_n_plus_1_calls do
         merge_requests_allowing_collaboration(branch_name).any? do |merge_request|
-          merge_request.can_be_merged_by?(user)
+          merge_request.can_be_merged_by?(user, skip_collaboration_check: true)
         end
       end
     end

changelogs/unreleased/security-kroki-arbitraryfile-read-write.yml

+---
+title: Fix arbitrary read/write in AsciiDoctor and Kroki gems
+merge_request:
+author:
+type: security

changelogs/unreleased/security-projects-branch-collaboration-loop.yml

+---
+title: Prevent infinite loop when checking if collaboration is allowed
+merge_request:
+author:
+type: security

config/initializers/asciidoctor_patch.rb

+# frozen_string_literal: true
+
+# Ensure that locked attributes can not be changed using a counter.
+# TODO: this can be removed once `asciidoctor` gem is > 2.0.12
+#       and https://github.com/asciidoctor/asciidoctor/issues/3939 is merged
+module Asciidoctor
+  module DocumentPatch
+    def counter(name, seed = nil)
+      return @parent_document.counter(name, seed) if @parent_document   # rubocop: disable Gitlab/ModuleWithInstanceVariables
+
+      unless attribute_locked? name
+        super
+      end
+    end
+  end
+end
+
+class Asciidoctor::Document
+  prepend Asciidoctor::DocumentPatch
+end

lib/gitlab/user_access.rb

@@ -11,10 +11,11 @@ module Gitlab
     attr_reader :user, :push_ability
     attr_accessor :container
 
-    def initialize(user, container: nil, push_ability: :push_code)
+    def initialize(user, container: nil, push_ability: :push_code, skip_collaboration_check: false)
       @user = user
       @container = container
       @push_ability = push_ability
+      @skip_collaboration_check = skip_collaboration_check
     end
 
     def can_do_action?(action)
@@ -87,6 +88,8 @@ module Gitlab
 
     private
 
+    attr_reader :skip_collaboration_check
+
     def can_push?
       user.can?(push_ability, container)
     end
@@ -98,6 +101,8 @@ module Gitlab
     end
 
     def branch_allows_collaboration_for?(ref)
+      return false if skip_collaboration_check
+
       # Checking for an internal project or group to prevent an infinite loop:
       # https://gitlab.com/gitlab-org/gitlab/issues/36805
       (!project.internal? && project.branch_allows_collaboration?(user, ref))

spec/lib/gitlab/asciidoc_spec.rb

@@ -92,6 +92,15 @@ module Gitlab
             expect(render(data[:input], context)).to include(data[:output])
           end
         end
+
+        it 'does not allow locked attributes to be overridden' do
+          input = <<~ADOC
+            {counter:max-include-depth:1234}
+            <|-- {max-include-depth}
+          ADOC
+
+          expect(render(input, {})).not_to include('1234')
+        end
       end
 
       context "images" do
@@ -543,6 +552,40 @@ module Gitlab
 
           expect(render(input, context)).to include(output.strip)
         end
+
+        it 'does not allow kroki-plantuml-include to be overridden' do
+          input = <<~ADOC
+            [plantuml, test="{counter:kroki-plantuml-include:/etc/passwd}", format="png"]
+            ....
+            class BlockProcessor
+
+            BlockProcessor <|-- {counter:kroki-plantuml-include}
+            ....
+          ADOC
+
+          output = <<~HTML
+            <div>
+            <div>
+            <a class=\"no-attachment-icon\" href=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==\" target=\"_blank\" rel=\"noopener noreferrer\"><img src=\"data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==\" alt=\"Diagram\" class=\"lazy\" data-src=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==\"></a>
+            </div>
+            </div>
+          HTML
+
+          expect(render(input, {})).to include(output.strip)
+        end
+
+        it 'does not allow kroki-server-url to be overridden' do
+          input = <<~ADOC
+            [plantuml, test="{counter:kroki-server-url:evilsite}", format="png"]
+            ....
+            class BlockProcessor
+
+            BlockProcessor
+            ....
+          ADOC
+
+          expect(render(input, {})).not_to include('evilsite')
+        end
       end
 
       context 'with Kroki and BlockDiag (additional format) enabled' do

spec/lib/gitlab/user_access_spec.rb

@@ -216,6 +216,15 @@ RSpec.describe Gitlab::UserAccess do
         expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
       end
     end
+
+    context 'when skip_collaboration_check is true' do
+      let(:access) { described_class.new(user, container: project, skip_collaboration_check: true) }
+
+      it 'does not call Project#branch_allows_collaboration?' do
+        expect(project).not_to receive(:branch_allows_collaboration?)
+        expect(access.can_push_to_branch?('master')).to be_falsey
+      end
+    end
   end
 
   describe '#can_create_tag?' do

spec/models/project_spec.rb

@@ -5291,6 +5291,64 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
+  describe '#branch_allows_collaboration?' do
+    context 'when there are open merge requests that have their source/target branches point to each other' do
+      let_it_be(:project) { create(:project, :repository) }
+      let_it_be(:developer) { create(:user) }
+      let_it_be(:reporter) { create(:user) }
+      let_it_be(:guest) { create(:user) }
+
+      before_all do
+        create(
+          :merge_request,
+          target_project: project,
+          target_branch: 'master',
+          source_project: project,
+          source_branch: 'merge-test',
+          allow_collaboration: true
+        )
+
+        create(
+          :merge_request,
+          target_project: project,
+          target_branch: 'merge-test',
+          source_project: project,
+          source_branch: 'master',
+          allow_collaboration: true
+        )
+
+        project.add_developer(developer)
+        project.add_reporter(reporter)
+        project.add_guest(guest)
+      end
+
+      shared_examples_for 'successful check' do
+        it 'does not go into an infinite loop' do
+          expect { project.branch_allows_collaboration?(user, 'master') }
+            .not_to raise_error
+        end
+      end
+
+      context 'when user is a developer' do
+        let(:user) { developer }
+
+        it_behaves_like 'successful check'
+      end
+
+      context 'when user is a reporter' do
+        let(:user) { reporter }
+
+        it_behaves_like 'successful check'
+      end
+
+      context 'when user is a guest' do
+        let(:user) { guest }
+
+        it_behaves_like 'successful check'
+      end
+    end
+  end
+
   context 'with cross project merge requests' do
     let(:user) { create(:user) }
     let(:target_project) { create(:project, :repository) }