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