Merge branch 'sh-handle-invalid-comparison' into 'master'

Reject invalid branch names in repository compare controller

Closes #51003

See merge request gitlab-org/gitlab-ce!22186

3 files changed, 46 insertions, 0 deletions

diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index c2df7b34f90..2917925947f 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -16,6 +16,8 @@ class Projects::CompareController < Projects::ApplicationController
   before_action :define_diff_notes_disabled, only: [:show, :diff_for_path]
   before_action :define_commits, only: [:show, :diff_for_path, :signatures]
   before_action :merge_request, only: [:index, :show]
+  # Validation
+  before_action :validate_refs!
 
   def index
   end
@@ -63,6 +65,21 @@ class Projects::CompareController < Projects::ApplicationController
 
   private
 
+  def valid_ref?(ref_name)
+    return true unless ref_name.present?
+
+    Gitlab::GitRefValidator.validate(ref_name)
+  end
+
+  def validate_refs!
+    valid = [head_ref, start_ref].map { |ref| valid_ref?(ref) }
+
+    return if valid.all?
+
+    flash[:alert] = "Invalid branch name"
+    redirect_to project_compare_index_path(@project)
+  end
+
   def compare
     return @compare if defined?(@compare)
 
diff --git a/changelogs/unreleased/sh-handle-invalid-comparison.yml b/changelogs/unreleased/sh-handle-invalid-comparison.yml
new file mode 100644
index 00000000000..30b5b3d8198
--- /dev/null
+++ b/changelogs/unreleased/sh-handle-invalid-comparison.yml
@@ -0,0 +1,5 @@
+---
+title: Reject invalid branch names in repository compare controller
+merge_request: 22186
+author:
+type: fixed
diff --git a/spec/controllers/projects/compare_controller_spec.rb b/spec/controllers/projects/compare_controller_spec.rb
index 8695aa826bb..17883d0fadd 100644
--- a/spec/controllers/projects/compare_controller_spec.rb
+++ b/spec/controllers/projects/compare_controller_spec.rb
@@ -97,6 +97,30 @@ describe Projects::CompareController do
         expect(assigns(:commits)).to eq([])
       end
     end
+
+    context 'when the target ref is invalid' do
+      let(:target_ref) { "master%' AND 2554=4423 AND '%'='" }
+      let(:source_ref) { "improve%2Fawesome" }
+
+      it 'shows a flash message and redirects' do
+        show_request
+
+        expect(flash[:alert]).to eq('Invalid branch name')
+        expect(response).to have_http_status(302)
+      end
+    end
+
+    context 'when the source ref is invalid' do
+      let(:source_ref) { "master%' AND 2554=4423 AND '%'='" }
+      let(:target_ref) { "improve%2Fawesome" }
+
+      it 'shows a flash message and redirects' do
+        show_request
+
+        expect(flash[:alert]).to eq('Invalid branch name')
+        expect(response).to have_http_status(302)
+      end
+    end
   end
 
   describe 'GET diff_for_path' do