diff options
6 files changed, 95 insertions, 1 deletions
diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 7a1bb9dd3ca..787d9db1192 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -138,7 +138,7 @@ = clipboard_button(text: source_branch, title: _('Copy branch name'), placement: "left", boundary: 'viewport') .sidebar-mr-source-branch.hide-collapsed %span - = _('Source branch: %{source_branch_open}%{source_branch}%{source_branch_close}').html_safe % { source_branch_open: "<cite class='ref-name' title='#{source_branch}'>".html_safe, source_branch_close: "</cite>".html_safe, source_branch: source_branch } + = _('Source branch: %{source_branch_open}%{source_branch}%{source_branch_close}').html_safe % { source_branch_open: "<cite class='ref-name' title='#{html_escape(source_branch)}'>".html_safe, source_branch_close: "</cite>".html_safe, source_branch: html_escape(source_branch) } = clipboard_button(text: source_branch, title: _('Copy branch name'), placement: "left", boundary: 'viewport') - if show_forwarding_email diff --git a/changelogs/unreleased/security-fix-xss-in-mr-sidebar.yml b/changelogs/unreleased/security-fix-xss-in-mr-sidebar.yml new file mode 100644 index 00000000000..a04c1038877 --- /dev/null +++ b/changelogs/unreleased/security-fix-xss-in-mr-sidebar.yml @@ -0,0 +1,5 @@ +--- +title: Fixed XSS in merge requests sidebar +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-sh-json-validator-open-uri-patch.yml b/changelogs/unreleased/security-sh-json-validator-open-uri-patch.yml new file mode 100644 index 00000000000..bf51ad66174 --- /dev/null +++ b/changelogs/unreleased/security-sh-json-validator-open-uri-patch.yml @@ -0,0 +1,5 @@ +--- +title: Disable arbitrary URI and file reads in JSON validator +merge_request: +author: +type: security diff --git a/config/initializers/json_validator_patch.rb b/config/initializers/json_validator_patch.rb new file mode 100644 index 00000000000..cb4158045ee --- /dev/null +++ b/config/initializers/json_validator_patch.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# This patches https://github.com/ruby-json-schema/json-schema/blob/765e6d8fdbfdaca1a42fa743f4621e757f9f6a03/lib/json-schema/validator.rb +# to address https://github.com/ruby-json-schema/json-schema/issues/148. +require 'json-schema' + +module JSON + class Validator + def initialize_data(data) + if @options[:parse_data] + if @options[:json] + data = self.class.parse(data) + elsif @options[:uri] + json_uri = Util::URI.normalized_uri(data) + data = self.class.parse(custom_open(json_uri)) + elsif data.is_a?(String) + begin + data = self.class.parse(data) + rescue JSON::Schema::JsonParseError + # Silently discard the error - use the data as-is + end + end + end + + JSON::Schema.stringify(data) + end + end +end diff --git a/spec/features/merge_request/user_views_open_merge_request_spec.rb b/spec/features/merge_request/user_views_open_merge_request_spec.rb index 9bda48a3ec5..5f99d762ecb 100644 --- a/spec/features/merge_request/user_views_open_merge_request_spec.rb +++ b/spec/features/merge_request/user_views_open_merge_request_spec.rb @@ -111,4 +111,21 @@ RSpec.describe 'User views an open merge request' do end end end + + context 'XSS source branch' do + let(:project) { create(:project, :public, :repository) } + let(:source_branch) { "'><iframe/srcdoc=''></iframe>" } + + before do + project.repository.create_branch(source_branch, "master") + + mr = create(:merge_request, source_project: project, target_project: project, source_branch: source_branch) + + visit(merge_request_path(mr)) + end + + it 'encodes branch name' do + expect(find('cite.ref-name')[:title]).to eq(source_branch) + end + end end diff --git a/spec/initializers/json_validator_patch_spec.rb b/spec/initializers/json_validator_patch_spec.rb new file mode 100644 index 00000000000..5d90364ae92 --- /dev/null +++ b/spec/initializers/json_validator_patch_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' +require 'rspec-parameterized' + +RSpec.describe 'JSON validator patch' do + using RSpec::Parameterized::TableSyntax + + let(:schema) { '{"format": "string"}' } + + subject { JSON::Validator.validate(schema, data) } + + context 'with invalid JSON' do + where(:data) do + [ + 'https://example.com', + '/tmp/test.txt' + ] + end + + with_them do + it 'does not attempt to open a file or URI' do + allow(File).to receive(:read).and_call_original + allow(URI).to receive(:open).and_call_original + expect(File).not_to receive(:read).with(data) + expect(URI).not_to receive(:open).with(data) + expect(subject).to be true + end + end + end + + context 'with valid JSON' do + let(:data) { %({ 'somekey': 'value' }) } + + it 'validates successfully' do + expect(subject).to be true + end + end +end |