Avoid exposing unaccessible repo data upon GFM processing

When post-processing relative links to absolute links
RelativeLinkFilter didn't take into consideration that
internal repository data could be exposed for users
that do not have repository access to the project.

This commit solves that by checking whether the user
can `download_code` at this repository, avoiding any
processing of this filter if the user can't.

Additionally, if we're processing for a group (
no project was given), we check if the user can
read it in order to expand the href as an extra.
That doesn't seem necessarily a breach now,
but an extra check doesn't hurt as after all
the user needs to be able to `read_group`.

4 files changed, 97 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 86f18679496..846a7d46aad 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/helpers/markup_helper_spec.rb b/spec/helpers/markup_helper_spec.rb
index f6e1720e113..1757ec8fa4d 100644
--- a/spec/helpers/markup_helper_spec.rb
+++ b/spec/helpers/markup_helper_spec.rb
@@ -65,6 +65,9 @@ describe MarkupHelper do
 
       describe 'inside a group' do
         before do
+          # Ensure the generated reference links aren't redacted
+          group.add_maintainer(user)
+
           helper.instance_variable_set(:@group, group)
           helper.instance_variable_set(:@project, nil)
         end
@@ -78,6 +81,9 @@ describe MarkupHelper do
         let(:project_in_group) { create(:project, group: group) }
 
         before do
+          # Ensure the generated reference links aren't redacted
+          project_in_group.add_maintainer(user)
+
           helper.instance_variable_set(:@group, group)
           helper.instance_variable_set(:@project, project_in_group)
         end
diff --git a/spec/lib/banzai/filter/relative_link_filter_spec.rb b/spec/lib/banzai/filter/relative_link_filter_spec.rb
index ecb83b6cb66..789530fbc56 100644
--- a/spec/lib/banzai/filter/relative_link_filter_spec.rb
+++ b/spec/lib/banzai/filter/relative_link_filter_spec.rb
@@ -7,6 +7,7 @@ describe Banzai::Filter::RelativeLinkFilter do
     contexts.reverse_merge!({
       commit:         commit,
       project:        project,
+      current_user:   user,
       group:          group,
       project_wiki:   project_wiki,
       ref:            ref,
@@ -33,7 +34,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' }
@@ -75,6 +77,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
@@ -282,6 +289,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 }
@@ -331,11 +369,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 }