diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-08-26 23:06:11 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-08-26 23:06:11 +0000 |
commit | 49cee93674e75cfa03d7457c1d22feb2c6606896 (patch) | |
tree | 8278644c616a81705ce82515c0b37a53d3c9c1a2 | |
parent | 3403437293d5559250a4176e0cc5998c3133e55c (diff) | |
parent | c559611a8bb1df04128ab3f6ccedc88d441b0d47 (diff) | |
download | gitlab-ce-49cee93674e75cfa03d7457c1d22feb2c6606896.tar.gz |
Merge branch 'security-exposed-default-branch-12-0' into '12-0-stable'
Avoid exposing unaccessible repo data upon GFM post processing
See merge request gitlab/gitlabhq!3384
-rw-r--r-- | changelogs/unreleased/security-exposed-default-branch.yml | 5 | ||||
-rw-r--r-- | lib/banzai/filter/relative_link_filter.rb | 16 | ||||
-rw-r--r-- | spec/lib/banzai/filter/relative_link_filter_spec.rb | 72 |
3 files changed, 91 insertions, 2 deletions
diff --git a/changelogs/unreleased/security-exposed-default-branch.yml b/changelogs/unreleased/security-exposed-default-branch.yml new file mode 100644 index 00000000000..bf32617ee8a --- /dev/null +++ b/changelogs/unreleased/security-exposed-default-branch.yml @@ -0,0 +1,5 @@ +--- +title: Avoid exposing unaccessible repo data upon GFM post processing +merge_request: +author: +type: security diff --git a/lib/banzai/filter/relative_link_filter.rb b/lib/banzai/filter/relative_link_filter.rb index bf84d7cddae..114b0cf5daf 100644 --- a/lib/banzai/filter/relative_link_filter.rb +++ b/lib/banzai/filter/relative_link_filter.rb @@ -9,6 +9,7 @@ module Banzai # Context options: # :commit # :group + # :current_user # :project # :project_wiki # :ref @@ -18,6 +19,7 @@ module Banzai def call return doc if context[:system_note] + return doc unless visible_to_user? @uri_types = {} clear_memoization(:linkable_files) @@ -166,6 +168,16 @@ module Banzai Gitlab.config.gitlab.relative_url_root.presence || '/' end + def visible_to_user? + if project + Ability.allowed?(current_user, :download_code, project) + elsif group + Ability.allowed?(current_user, :read_group, group) + else # Objects detached from projects or groups, e.g. Personal Snippets. + true + end + end + def ref context[:ref] || project.default_branch end @@ -178,6 +190,10 @@ module Banzai context[:project] end + def current_user + context[:current_user] + end + def repository @repository ||= project&.repository end diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb index a714fa50f5f..0fe070608d4 100644 --- a/spec/lib/banzai/filter/relative_link_filter_spec.rb +++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb @@ -5,6 +5,7 @@ describe Banzai::Filter::RelativeLinkFilter do contexts.reverse_merge!({ commit: commit, project: project, + current_user: user, group: group, project_wiki: project_wiki, ref: ref, @@ -31,7 +32,8 @@ describe Banzai::Filter::RelativeLinkFilter do %(<div>#{element}</div>) end - let(:project) { create(:project, :repository) } + let(:project) { create(:project, :repository, :public) } + let(:user) { create(:user) } let(:group) { nil } let(:project_path) { project.full_path } let(:ref) { 'markdown' } @@ -73,6 +75,11 @@ describe Banzai::Filter::RelativeLinkFilter do include_examples :preserve_unchanged end + context 'without project repository access' do + let(:project) { create(:project, :repository, repository_access_level: ProjectFeature::PRIVATE) } + include_examples :preserve_unchanged + end + it 'does not raise an exception on invalid URIs' do act = link("://foo") expect { filter(act) }.not_to raise_error @@ -280,6 +287,37 @@ describe Banzai::Filter::RelativeLinkFilter do let(:relative_path) { "/#{project.full_path}#{upload_path}" } context 'to a project upload' do + context 'without project repository access' do + let(:project) { create(:project, :repository, repository_access_level: ProjectFeature::PRIVATE) } + + it 'does not rebuild relative URL for a link' do + doc = filter(link(upload_path)) + expect(doc.at_css('a')['href']).to eq(upload_path) + + doc = filter(nested(link(upload_path))) + expect(doc.at_css('a')['href']).to eq(upload_path) + end + + it 'does not rebuild relative URL for an image' do + doc = filter(image(upload_path)) + expect(doc.at_css('img')['src']).to eq(upload_path) + + doc = filter(nested(image(upload_path))) + expect(doc.at_css('img')['src']).to eq(upload_path) + end + + context 'with an absolute URL' do + let(:absolute_path) { Gitlab.config.gitlab.url + relative_path } + let(:only_path) { false } + + it 'does not rewrite the link' do + doc = filter(link(upload_path)) + + expect(doc.at_css('a')['href']).to eq(upload_path) + end + end + end + context 'with an absolute URL' do let(:absolute_path) { Gitlab.config.gitlab.url + relative_path } let(:only_path) { false } @@ -329,11 +367,41 @@ describe Banzai::Filter::RelativeLinkFilter do end context 'to a group upload' do - let(:upload_link) { link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg') } + let(:upload_path) { '/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg' } + let(:upload_link) { link(upload_path) } let(:group) { create(:group) } let(:project) { nil } let(:relative_path) { "/groups/#{group.full_path}/-/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg" } + context 'without group read access' do + let(:group) { create(:group, :private) } + + it 'does not rewrite the link' do + doc = filter(upload_link) + + expect(doc.at_css('a')['href']).to eq(upload_path) + end + + it 'does not rewrite the link for subgroup' do + group.update!(parent: create(:group)) + + doc = filter(upload_link) + + expect(doc.at_css('a')['href']).to eq(upload_path) + end + + context 'with an absolute URL' do + let(:absolute_path) { Gitlab.config.gitlab.url + relative_path } + let(:only_path) { false } + + it 'does not rewrite the link' do + doc = filter(upload_link) + + expect(doc.at_css('a')['href']).to eq(upload_path) + end + end + end + context 'with an absolute URL' do let(:absolute_path) { Gitlab.config.gitlab.url + relative_path } let(:only_path) { false } |