debian-mirror-gitlab/debian/patches/CVE-2019-19257.patch

From debb36496b4805beae28262fbb24a692018178e2 Mon Sep 17 00:00:00 2001
From: Kerri Miller <kerrizor@kerrizor.com>
Date: Fri, 25 Oct 2019 07:46:40 -0500
Subject: [PATCH] Restrict branches visible to guests in Issue feed

Notes related to branch creation should not be shown in an issue's
activity feed when the user doesn't have access to :download_code.
---
 app/models/note.rb                            | 15 ++++-
 ...er-related-branches-from-activity-feed.yml |  6 ++
 .../projects/issues_controller_spec.rb        | 37 +++++++++++
 spec/models/note_spec.rb                      | 64 +++++++++++++++++++
 4 files changed, 121 insertions(+), 1 deletion(-)
 create mode 100644 changelogs/unreleased/security-filter-related-branches-from-activity-feed.yml

--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -40,6 +40,10 @@
 
   redact_field :note
 
+  TYPES_RESTRICTED_BY_ABILITY = {
+    branch: :download_code
+  }.freeze
+
   # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with notes.
   # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10392/diffs#note_28719102
   alias_attribute :last_edited_at, :updated_at
@@ -333,7 +337,7 @@
   end
 
   def visible_for?(user)
-    !cross_reference_not_visible_for?(user)
+    !cross_reference_not_visible_for?(user) && system_note_viewable_by?(user)
   end
 
   def award_emoji?
@@ -485,6 +489,15 @@
 
   private
 
+  def system_note_viewable_by?(user)
+    return true unless system_note_metadata
+
+    restriction = TYPES_RESTRICTED_BY_ABILITY[system_note_metadata.action.to_sym]
+    return Ability.allowed?(user, restriction, project) if restriction
+
+    true
+  end
+
   def keep_around_commit
     project.repository.keep_around(self.commit_id)
   end
--- /dev/null
+++ b/changelogs/unreleased/security-filter-related-branches-from-activity-feed.yml
@@ -0,0 +1,6 @@
+---
+title: Remove notes regarding Related Branches from Issue activity feeds for guest
+  users
+merge_request:
+author:
+type: security
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -1343,6 +1343,43 @@
           expect { get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid } }.not_to exceed_query_limit(control_count)
         end
       end
+
+      context 'private project' do
+        let!(:branch_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
+        let!(:commit_note) { create(:discussion_note_on_issue, :system, noteable: issue, project: project) }
+        let!(:branch_note_meta) { create(:system_note_metadata, note: branch_note, action: "branch") }
+        let!(:commit_note_meta) { create(:system_note_metadata, note: commit_note, action: "commit") }
+
+        context 'user is allowed access' do
+          before do
+            project.add_user(user, :maintainer)
+          end
+
+          it 'displays all available notes' do
+            get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
+
+            expect(json_response.length).to eq(3)
+          end
+        end
+
+        context 'user is a guest' do
+          let(:json_response_note_ids) do
+            json_response.collect { |discussion| discussion["notes"] }.flatten
+              .collect { |note| note["id"].to_i }
+          end
+
+          before do
+            project.add_guest(user)
+          end
+
+          it 'does not display notes w/type listed in TYPES_RESTRICTED_BY_ACCESS_LEVEL' do
+            get :discussions, params: { namespace_id: project.namespace, project_id: project, id: issue.iid }
+
+            expect(json_response.length).to eq(2)
+            expect(json_response_note_ids).not_to include(branch_note.id)
+          end
+        end
+      end
     end
   end
 
--- a/spec/models/note_spec.rb
+++ b/spec/models/note_spec.rb
@@ -246,6 +246,70 @@
     end
   end
 
+  describe "#visible_for?" do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:note) { create(:note) }
+    let(:user) { create(:user) }
+
+    where(:cross_reference_visible, :system_note_viewable, :result) do
+      true  | true  | false
+      false | true  | true
+      false | false | false
+    end
+
+    with_them do
+      it "returns expected result" do
+        expect(note).to receive(:cross_reference_not_visible_for?).and_return(cross_reference_visible)
+
+        unless cross_reference_visible
+          expect(note).to receive(:system_note_viewable_by?)
+            .with(user).and_return(system_note_viewable)
+        end
+
+        expect(note.visible_for?(user)).to eq result
+      end
+    end
+  end
+
+  describe "#system_note_viewable_by?(user)" do
+    let(:note) { create(:note) }
+    let(:user) { create(:user) }
+    let!(:metadata) { create(:system_note_metadata, note: note, action: "branch") }
+
+    context "when system_note_metadata is not present" do
+      it "returns true" do
+        expect(note).to receive(:system_note_metadata).and_return(nil)
+
+        expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+      end
+    end
+
+    context "system_note_metadata isn't of type 'branch'" do
+      before do
+        metadata.action = "not_a_branch"
+      end
+
+      it "returns true" do
+        expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+      end
+    end
+
+    context "user doesn't have :download_code ability" do
+      it "returns false" do
+        expect(note.send(:system_note_viewable_by?, user)).to be_falsey
+      end
+    end
+
+    context "user has the :download_code ability" do
+      it "returns true" do
+        expect(Ability).to receive(:allowed?).with(user, :download_code, note.project).and_return(true)
+
+        expect(note.send(:system_note_viewable_by?, user)).to be_truthy
+      end
+    end
+  end
+
   describe "cross_reference_not_visible_for?" do
     let(:private_user)    { create(:user) }
     let(:private_project) { create(:project, namespace: private_user.namespace) { |p| p.add_maintainer(private_user) } }