summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-03-30 22:43:19 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-03-30 22:43:29 +0000
commit7608c39220287b873b0c06f996da724241e31f33 (patch)
tree2ddfb9df7a11286eb45d66d164b34b53761dee42
parent164ecc9a6349fd6a33960e4fb2f841fde3bd49b8 (diff)
downloadgitlab-ce-7608c39220287b873b0c06f996da724241e31f33.tar.gz
Add latest changes from gitlab-org/security/gitlab@13-9-stable-ee
-rw-r--r--app/views/shared/issuable/_sidebar.html.haml2
-rw-r--r--changelogs/unreleased/security-fix-xss-in-mr-sidebar.yml5
-rw-r--r--changelogs/unreleased/security-sh-json-validator-open-uri-patch.yml5
-rw-r--r--config/initializers/json_validator_patch.rb28
-rw-r--r--lib/gitlab/markdown_cache.rb2
-rw-r--r--spec/features/merge_request/user_views_open_merge_request_spec.rb17
-rw-r--r--spec/initializers/json_validator_patch_spec.rb39
7 files changed, 96 insertions, 2 deletions
diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml
index f26f4adc19a..27fcaeb37ac 100644
--- a/app/views/shared/issuable/_sidebar.html.haml
+++ b/app/views/shared/issuable/_sidebar.html.haml
@@ -143,7 +143,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/lib/gitlab/markdown_cache.rb b/lib/gitlab/markdown_cache.rb
index 36e9a6ccef6..3ec5f2339b5 100644
--- a/lib/gitlab/markdown_cache.rb
+++ b/lib/gitlab/markdown_cache.rb
@@ -3,7 +3,7 @@
module Gitlab
module MarkdownCache
# Increment this number every time the renderer changes its output
- CACHE_COMMONMARK_VERSION = 26
+ CACHE_COMMONMARK_VERSION = 27
CACHE_COMMONMARK_VERSION_START = 10
BaseError = Class.new(StandardError)
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) { "&#39;&gt;&lt;iframe/srcdoc=&#39;&#39;&gt;&lt;/iframe&gt;" }
+
+ 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