summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-10-24 18:53:03 +0000
committerGitLab Release Tools Bot <robert+release-tools@gitlab.com>2019-10-24 18:53:03 +0000
commitddfb716076bd94f02293505881f7a9333d3a5da4 (patch)
tree260cf1c6869e748b183c3172a34fa485f00c91bd
parent203f5b43734cbb320bcfdd1f3929f4fb1862c8dc (diff)
parent99c8d262a73eb5f8fef6ab7e1aeb961e16d25c28 (diff)
downloadgitlab-ce-ddfb716076bd94f02293505881f7a9333d3a5da4.tar.gz
Merge branch 'security-65756-ex-admin-attacker-can-comment-in-internalsecurity-65756-ex-admin-attacker-can-comment-in-internal-12-4' into '12-4-stable'
Improper access control allows the attacker to comment in internal commit after they are no longer admin See merge request gitlab/gitlabhq!3497
-rw-r--r--app/policies/commit_policy.rb1
-rw-r--r--changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml5
-rw-r--r--spec/policies/commit_policy_spec.rb48
3 files changed, 42 insertions, 12 deletions
diff --git a/app/policies/commit_policy.rb b/app/policies/commit_policy.rb
index 4d4f0ba9267..4b358c45ec2 100644
--- a/app/policies/commit_policy.rb
+++ b/app/policies/commit_policy.rb
@@ -4,4 +4,5 @@ class CommitPolicy < BasePolicy
delegate { @subject.project }
rule { can?(:download_code) }.enable :read_commit
+ rule { ~can?(:read_commit) }.prevent :create_note
end
diff --git a/changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml b/changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml
new file mode 100644
index 00000000000..3d9f480ba11
--- /dev/null
+++ b/changelogs/unreleased/security-65756-ex-admin-attacker-can-comment-in-internal.yml
@@ -0,0 +1,5 @@
+---
+title: Disallow unprivileged users from commenting on private repository commits
+merge_request:
+author:
+type: security
diff --git a/spec/policies/commit_policy_spec.rb b/spec/policies/commit_policy_spec.rb
index 41f6fb08426..40183f51e9e 100644
--- a/spec/policies/commit_policy_spec.rb
+++ b/spec/policies/commit_policy_spec.rb
@@ -8,28 +8,42 @@ describe CommitPolicy do
let(:commit) { project.repository.head_commit }
let(:policy) { described_class.new(user, commit) }
+ shared_examples 'can read commit and create a note' do
+ it 'can read commit' do
+ expect(policy).to be_allowed(:read_commit)
+ end
+
+ it 'can create a note' do
+ expect(policy).to be_allowed(:create_note)
+ end
+ end
+
+ shared_examples 'cannot read commit nor create a note' do
+ it 'can not read commit' do
+ expect(policy).to be_disallowed(:read_commit)
+ end
+
+ it 'can not create a note' do
+ expect(policy).to be_disallowed(:create_note)
+ end
+ end
+
context 'when project is public' do
let(:project) { create(:project, :public, :repository) }
- it 'can read commit and create a note' do
- expect(policy).to be_allowed(:read_commit)
- end
+ it_behaves_like 'can read commit and create a note'
context 'when repository access level is private' do
let(:project) { create(:project, :public, :repository, :repository_private) }
- it 'can not read commit and create a note' do
- expect(policy).to be_disallowed(:read_commit)
- end
+ it_behaves_like 'cannot read commit nor create a note'
context 'when the user is a project member' do
before do
project.add_developer(user)
end
- it 'can read commit and create a note' do
- expect(policy).to be_allowed(:read_commit)
- end
+ it_behaves_like 'can read commit and create a note'
end
end
end
@@ -37,9 +51,7 @@ describe CommitPolicy do
context 'when project is private' do
let(:project) { create(:project, :private, :repository) }
- it 'can not read commit and create a note' do
- expect(policy).to be_disallowed(:read_commit)
- end
+ it_behaves_like 'cannot read commit nor create a note'
context 'when the user is a project member' do
before do
@@ -50,6 +62,18 @@ describe CommitPolicy do
expect(policy).to be_allowed(:read_commit)
end
end
+
+ context 'when the user is a guest' do
+ before do
+ project.add_guest(user)
+ end
+
+ it_behaves_like 'cannot read commit nor create a note'
+
+ it 'cannot download code' do
+ expect(policy).to be_disallowed(:download_code)
+ end
+ end
end
end
end