Commit ddfa9cb2 authored by Robert Speicher's avatar Robert Speicher

Merge branch 'master' of dev.gitlab.org:gitlab/gitlab-ee

parents 37452de8 e0a6a21a
Please view this file on the master branch, on stable branches it's out of date.
## 13.10.1 (2021-03-31)
### Security (1 change)
- Escape HTML on scoped labels tooltip.
## 13.10.0 (2021-03-22)
### Removed (1 change)
......@@ -167,6 +174,13 @@ Please view this file on the master branch, on stable branches it's out of date.
- Delete redirect files. !56169
## 13.9.5 (2021-03-31)
### Security (1 change)
- Escape HTML on scoped labels tooltip.
## 13.9.4 (2021-03-17)
- No changes.
......@@ -337,6 +351,13 @@ Please view this file on the master branch, on stable branches it's out of date.
- Review UI text - repo push rules settings. !52797
## 13.8.7 (2021-03-31)
### Security (1 change)
- Escape HTML on scoped labels tooltip.
## 13.8.6 (2021-03-17)
- No changes.
......
......@@ -2,6 +2,28 @@
documentation](doc/development/changelog.md) for instructions on adding your own
entry.
## 13.10.1 (2021-03-31)
### Security (6 changes)
- Leave pool repository on fork unlinking.
- Fixed XSS in merge requests sidebar.
- Fix arbitrary read/write in AsciiDoctor and Kroki gems.
- Prevent infinite loop when checking if collaboration is allowed.
- Disable arbitrary URI and file reads in JSON validator.
- Require POST request to trigger system hooks.
### Removed (1 change)
- Make HipChat project service do nothing. !57434
### Other (3 changes)
- Remove direct mimemagic dependency. !57387
- Refactor MimeMagic calls to new MimeType class. !57421
- Switch to using a fake mimemagic gem. !57443
## 13.10.0 (2021-03-22)
### Security (3 changes)
......@@ -529,6 +551,28 @@ entry.
- Convert mattermost alert to pajamas. !56556
## 13.9.5 (2021-03-31)
### Security (6 changes)
- Leave pool repository on fork unlinking.
- Fixed XSS in merge requests sidebar.
- Fix arbitrary read/write in AsciiDoctor and Kroki gems.
- Prevent infinite loop when checking if collaboration is allowed.
- Disable arbitrary URI and file reads in JSON validator.
- Require POST request to trigger system hooks.
### Removed (1 change)
- Make HipChat project service do nothing. !57434
### Other (3 changes)
- Remove direct mimemagic dependency. !57387
- Refactor MimeMagic calls to new MimeType class. !57421
- Switch to using a fake mimemagic gem. !57443
## 13.9.4 (2021-03-17)
### Security (1 change)
......@@ -1144,6 +1188,27 @@ entry.
- Apply new GitLab UI for buttons in pipeline schedules.
## 13.8.7 (2021-03-31)
### Security (5 changes)
- Fixed XSS in merge requests sidebar.
- Leave pool repository on fork unlinking.
- Fix arbitrary read/write in AsciiDoctor and Kroki gems.
- Prevent infinite loop when checking if collaboration is allowed.
- Require POST request to trigger system hooks.
### Removed (1 change)
- Make HipChat project service do nothing. !57434
### Other (3 changes)
- Remove direct mimemagic dependency. !57387
- Refactor MimeMagic calls to new MimeType class. !57421
- Switch to using a fake mimemagic gem. !57443
## 13.8.6 (2021-03-17)
### Security (1 change)
......
......@@ -1350,8 +1350,8 @@ class MergeRequest < ApplicationRecord
has_no_commits? || branch_missing? || cannot_be_merged?
end
def can_be_merged_by?(user)
access = ::Gitlab::UserAccess.new(user, container: project)
def can_be_merged_by?(user, skip_collaboration_check: false)
access = ::Gitlab::UserAccess.new(user, container: project, skip_collaboration_check: skip_collaboration_check)
access.can_update_branch?(target_branch)
end
......
......@@ -2711,7 +2711,7 @@ class Project < ApplicationRecord
# Issue for N+1: https://gitlab.com/gitlab-org/gitlab-foss/issues/49322
Gitlab::GitalyClient.allow_n_plus_1_calls do
merge_requests_allowing_collaboration(branch_name).any? do |merge_request|
merge_request.can_be_merged_by?(user)
merge_request.can_be_merged_by?(user, skip_collaboration_check: true)
end
end
end
......
......@@ -32,6 +32,8 @@ module Projects
if fork_network = @project.root_of_fork_network
fork_network.update(root_project: nil, deleted_root_project_name: @project.full_name)
end
@project.leave_pool_repository
end
# rubocop: disable Cop/InBatches
......
......@@ -138,7 +138,7 @@
= clipboard_button(text: source_branch, title: _('Copy branch name'), placement: "left", boundary: 'viewport')
.gl-display-flex.gl-align-items-center.gl-justify-content-space-between.gl-mb-2.hide-collapsed
%span.gl-overflow-hidden.gl-text-overflow-ellipsis.gl-white-space-nowrap
= _('Source branch: %{source_branch_open}%{source_branch}%{source_branch_close}').html_safe % { source_branch_open: "<span class='gl-font-monospace' title='#{source_branch}'>".html_safe, source_branch_close: "</span>".html_safe, source_branch: source_branch }
= _('Source branch: %{source_branch_open}%{source_branch}%{source_branch_close}').html_safe % { source_branch_open: "<span class='gl-font-monospace' data-testid='ref-name' title='#{html_escape(source_branch)}'>".html_safe, source_branch_close: "</span>".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
......
---
title: Switch to using a fake mimemagic gem
merge_request: 57443
author:
type: other
---
title: Refactor MimeMagic calls to new MimeType class
merge_request: 57421
author:
type: other
---
title: Remove direct mimemagic dependency
merge_request: 57387
author:
type: other
---
title: Make HipChat project service do nothing
merge_request: 57434
author:
type: removed
# frozen_string_literal: true
# Ensure that locked attributes can not be changed using a counter.
# TODO: this can be removed once `asciidoctor` gem is > 2.0.12
# and https://github.com/asciidoctor/asciidoctor/issues/3939 is merged
module Asciidoctor
module DocumentPatch
def counter(name, seed = nil)
return @parent_document.counter(name, seed) if @parent_document # rubocop: disable Gitlab/ModuleWithInstanceVariables
unless attribute_locked? name
super
end
end
end
end
class Asciidoctor::Document
prepend Asciidoctor::DocumentPatch
end
......@@ -88,7 +88,7 @@ Example response:
## Test system hook
```plaintext
GET /hooks/:id
POST /hooks/:id
```
| Attribute | Type | Required | Description |
......@@ -98,7 +98,7 @@ GET /hooks/:id
Example request:
```shell
curl --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/hooks/2"
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/hooks/1"
```
Example response:
......
......@@ -5,6 +5,10 @@ module EE
extend ActiveSupport::Concern
prepended do
condition(:is_author) do
@user && @subject.author_id == @user.id
end
rule { can?(:read_issue) }.policy do
enable :read_issuable_metric_image
end
......@@ -12,6 +16,15 @@ module EE
rule { can?(:create_issue) & can?(:update_issue) }.policy do
enable :upload_issuable_metric_image
end
rule { is_author | can?(:create_issue) & can?(:update_issue) }.policy do
enable :destroy_issuable_metric_image
end
rule { ~is_project_member }.policy do
prevent :upload_issuable_metric_image
prevent :destroy_issuable_metric_image
end
end
end
end
---
title: Fix permissions for modifying issue metric images
merge_request:
author:
type: security
......@@ -79,6 +79,9 @@ module EE
end
delete ':metric_image_id' do
issue = find_project_issue(params[:issue_iid])
authorize!(:destroy_issuable_metric_image, issue)
metric_image = issue.metric_images.find_by_id(params[:metric_image_id])
render_api_error!('Metric image not found', 404) unless metric_image
......@@ -93,12 +96,6 @@ module EE
end
helpers do
include ::API::Helpers::Packages::BasicAuthHelpers
def project
authorized_user_project
end
def max_file_size_exceeded?
params[:file].size > ::IssuableMetricImage::MAX_FILE_SIZE
end
......
......@@ -10,12 +10,18 @@ module EE
def data_attributes_for(text, parent, object, link_content: false, link_reference: false)
return super unless object.scoped_label?
# Enabling HTML tooltips for scoped labels here but we do not need to do any additional
# escaping because the label's tooltips are already stripped of dangerous HTML
# Enabling HTML tooltips for scoped labels here and additional escaping is done in `object_link_title`
super.merge!(
html: true
)
end
override :object_link_title
def object_link_title(object, matches)
return super unless object.scoped_label?
ERB::Util.html_escape(super)
end
end
end
end
......
......@@ -5,9 +5,10 @@ require 'spec_helper'
RSpec.describe Banzai::Filter::LabelReferenceFilter do
include FilterSpecHelper
let(:project) { create(:project, :public, name: 'sample-project') }
let(:label) { create(:label, name: 'label', project: project) }
let(:scoped_label) { create(:label, name: 'key::value', project: project) }
let(:project) { create(:project, :public, name: 'sample-project') }
let(:label) { create(:label, name: 'label', project: project) }
let(:scoped_description) { 'xss <script>alert("scriptAlert");</script> &<a>lt;svg id=&quot;svgId&quot;&gt;&lt;/svg&gt;' }
let(:scoped_label) { create(:label, name: 'key::value', project: project, description: scoped_description) }
context 'with scoped labels enabled' do
before do
......@@ -24,6 +25,10 @@ RSpec.describe Banzai::Filter::LabelReferenceFilter do
it 'renders HTML tooltips' do
expect(doc.at_css('.gl-label-scoped a').attr('data-html')).to eq('true')
end
it "escapes HTML in the label's title" do
expect(doc.at_css('.gl-label-scoped a').attr('title')).to include('xss &lt;svg id="svgId"&gt;')
end
end
context 'with a common label' do
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe IssuablePolicy, models: true do
let(:non_member) { create(:user) }
let(:guest) { create(:user) }
let(:reporter) { create(:user) }
let(:guest_issue) { create(:issue, project: project, author: guest) }
let(:reporter_issue) { create(:issue, project: project, author: reporter) }
before do
project.add_guest(guest)
project.add_reporter(reporter)
end
def permissions(user, issue)
described_class.new(user, issue)
end
describe '#rules' do
context 'in a public project' do
let_it_be(:project) { create(:project, :public) }
let_it_be(:issue) { create(:issue, project: project) }
it 'disallows non-members from creating and deleting metric images' do
expect(permissions(non_member, issue)).to be_allowed(:read_issuable_metric_image)
expect(permissions(non_member, issue)).to be_disallowed(:upload_issuable_metric_image, :destroy_issuable_metric_image)
end
it 'allows guests to read, create metric images, and delete them in their own issues' do
expect(permissions(guest, issue)).to be_allowed(:read_issuable_metric_image)
expect(permissions(guest, issue)).to be_disallowed(:upload_issuable_metric_image, :destroy_issuable_metric_image)
expect(permissions(guest, guest_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :destroy_issuable_metric_image)
end
it 'allows reporters to create and delete metric images' do
expect(permissions(reporter, issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :destroy_issuable_metric_image)
expect(permissions(reporter, reporter_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :destroy_issuable_metric_image)
end
end
context 'in a private project' do
let_it_be(:project) { create(:project, :private) }
let_it_be(:issue) { create(:issue, project: project) }
it 'disallows non-members from creating and deleting metric images' do
expect(permissions(non_member, issue)).to be_disallowed(:read_issuable_metric_image, :upload_issuable_metric_image, :destroy_issuable_metric_image)
end
it 'allows guests to read metric images, and create + delete in their own issues' do
expect(permissions(guest, issue)).to be_allowed(:read_issuable_metric_image)
expect(permissions(guest, issue)).to be_disallowed(:upload_issuable_metric_image, :destroy_issuable_metric_image)
expect(permissions(guest, guest_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :destroy_issuable_metric_image)
end
it 'allows reporters to create and delete metric images' do
expect(permissions(reporter, issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :destroy_issuable_metric_image)
expect(permissions(reporter, reporter_issue)).to be_allowed(:read_issuable_metric_image, :upload_issuable_metric_image, :destroy_issuable_metric_image)
end
end
end
end
......@@ -705,7 +705,7 @@ RSpec.describe API::Issues, :mailer do
using RSpec::Parameterized::TableSyntax
let_it_be(:project) do
create(:project, :private, creator_id: user.id, namespace: user.namespace)
create(:project, :public, creator_id: user.id, namespace: user.namespace)
end
let!(:image) { create(:issuable_metric_image, issue: issue) }
......@@ -722,6 +722,15 @@ RSpec.describe API::Issues, :mailer do
end
shared_examples 'unauthorized_delete' do
it 'cannot delete the metric image' do
subject
expect(response).to have_gitlab_http_status(:forbidden)
expect(image.reload).to eq(image)
end
end
shared_examples 'not_found' do
it 'cannot delete the metric image' do
subject
......@@ -734,9 +743,9 @@ RSpec.describe API::Issues, :mailer do
:not_member | false | false | :unauthorized_delete
:not_member | true | false | :unauthorized_delete
:not_member | true | true | :unauthorized_delete
:guest | false | true | :unauthorized_delete
:guest | false | true | :not_found
:guest | false | false | :unauthorized_delete
:guest | true | false | :can_delete_metric_image
:guest | false | false | :can_delete_metric_image
:reporter | true | false | :can_delete_metric_image
:reporter | false | false | :can_delete_metric_image
end
......
......@@ -47,7 +47,7 @@ module API
params do
requires :id, type: Integer, desc: 'The ID of the system hook'
end
get ":id" do
post ":id" do
hook = SystemHook.find(params[:id])
data = {
event_name: "project_create",
......
......@@ -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)
......
......@@ -11,10 +11,11 @@ module Gitlab
attr_reader :user, :push_ability
attr_accessor :container
def initialize(user, container: nil, push_ability: :push_code)
def initialize(user, container: nil, push_ability: :push_code, skip_collaboration_check: false)
@user = user
@container = container
@push_ability = push_ability
@skip_collaboration_check = skip_collaboration_check
end
def can_do_action?(action)
......@@ -87,6 +88,8 @@ module Gitlab
private
attr_reader :skip_collaboration_check
def can_push?
user.can?(push_ability, container)
end
......@@ -98,6 +101,8 @@ module Gitlab
end
def branch_allows_collaboration_for?(ref)
return false if skip_collaboration_check
# Checking for an internal project or group to prevent an infinite loop:
# https://gitlab.com/gitlab-org/gitlab/issues/36805
(!project.internal? && project.branch_allows_collaboration?(user, ref))
......
......@@ -6,7 +6,7 @@ FactoryBot.define do
state { :none }
before(:create) do |pool|
pool.source_project = create(:project, :repository)
pool.source_project ||= create(:project, :repository)
pool.source_project.update!(pool_repository: pool)
end
......
......@@ -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("[data-testid='ref-name']")[:title]).to eq(source_branch)
end
end
end
......@@ -92,6 +92,15 @@ module Gitlab
expect(render(data[:input], context)).to include(data[:output])
end
end
it 'does not allow locked attributes to be overridden' do
input = <<~ADOC
{counter:max-include-depth:1234}
<|-- {max-include-depth}
ADOC
expect(render(input, {})).not_to include('1234')
end
end
context "images" do
......@@ -543,6 +552,40 @@ module Gitlab
expect(render(input, context)).to include(output.strip)
end
it 'does not allow kroki-plantuml-include to be overridden' do
input = <<~ADOC
[plantuml, test="{counter:kroki-plantuml-include:/etc/passwd}", format="png"]
....
class BlockProcessor
BlockProcessor <|-- {counter:kroki-plantuml-include}
....
ADOC
output = <<~HTML
<div>
<div>
<a class=\"no-attachment-icon\" href=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==\" target=\"_blank\" rel=\"noopener noreferrer\"><img src=\"data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==\" alt=\"Diagram\" class=\"lazy\" data-src=\"https://kroki.io/plantuml/png/eNpLzkksLlZwyslPzg4oyk9OLS7OL-LiQuUr2NTo6ipUJ-eX5pWkFlllF-VnZ-oW5CTmlZTm5uhm5iXnlKak1gIABQEb8A==\"></a>
</div>
</div>
HTML
expect(render(input, {})).to include(output.strip)
end
it 'does not allow kroki-server-url to be overridden' do
input = <<~ADOC
[plantuml, test="{counter:kroki-server-url:evilsite}", format="png"]
....
class BlockProcessor
BlockProcessor
....
ADOC
expect(render(input, {})).not_to include('evilsite')
end
end
context 'with Kroki and BlockDiag (additional format) enabled' do
......
......@@ -216,6 +216,15 @@ RSpec.describe Gitlab::UserAccess do
expect(access.can_merge_to_branch?(@branch.name)).to be_falsey
end
end
context 'when skip_collaboration_check is true' do
let(:access) { described_class.new(user, container: project, skip_collaboration_check: true) }
it 'does not call Project#branch_allows_collaboration?' do
expect(project).not_to receive(:branch_allows_collaboration?)
expect(access.can_push_to_branch?('master')).to be_falsey
end
end
end
describe '#can_create_tag?' do
......
......@@ -5319,6 +5319,64 @@ RSpec.describe Project, factory_default: :keep do
end
end
describe '#branch_allows_collaboration?' do
context 'when there are open merge requests that have their source/target branches point to each other' do
let_it_be(:project) { create(:project, :repository) }
let_it_be(:developer) { create(:user) }
let_it_be(:reporter) { create(:user) }
let_it_be(:guest) { create(:user) }
before_all do
create(
:merge_request,
target_project: project,
target_branch: 'master',
source_project: project,
source_branch: 'merge-test',
allow_collaboration: true
)
create(
:merge_request,
target_project: project,
target_branch: 'merge-test',
source_project: project,
source_branch: 'master',
allow_collaboration: true
)
project.add_developer(developer)
project.add_reporter(reporter)
project.add_guest(guest)
end
shared_examples_for 'successful check' do
it 'does not go into an infinite loop' do
expect { project.branch_allows_collaboration?(user, 'master') }
.not_to raise_error
end
end
context 'when user is a developer' do
let(:user) { developer }
it_behaves_like 'successful check'
end
context 'when user is a reporter' do
let(:user) { reporter }
it_behaves_like 'successful check'
end
context 'when user is a guest' do
let(:user) { guest }
it_behaves_like 'successful check'
end
end
end
context 'with cross project merge requests' do
let(:user) { create(:user) }
let(:target_project) { create(:project, :repository) }
......
......@@ -103,15 +103,15 @@ RSpec.describe API::SystemHooks do
end
end
describe "GET /hooks/:id" do
it "returns hook by id" do
get api("/hooks/#{hook.id}", admin)
expect(response).to have_gitlab_http_status(:ok)
describe 'POST /hooks/:id' do
it "returns and trigger hook by id" do
post api("/hooks/#{hook.id}", admin)
expect(response).to have_gitlab_http_status(:created)
expect(json_response['event_name']).to eq('project_create')
end
it "returns 404 on failure" do
get api("/hooks/404", admin)
post api("/hooks/404", admin)
expect(response).to have_gitlab_http_status(:not_found)
end
end
......
......@@ -403,7 +403,7 @@ RSpec.describe Projects::ForkService do
end
context 'when forking with object pools' do
let(:fork_from_project) { create(:project, :public) }
let(:fork_from_project) { create(:project, :repository, :public) }
let(:forker) { create(:user) }
context 'when no pool exists' do
......
......@@ -207,6 +207,17 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin
end
end
context 'a project with pool repository' do
let(:project) { create(:project, :public, :repository) }
let!(:pool_repository) { create(:pool_repository, :ready, source_project: project) }
subject { described_class.new(project, user) }
it 'when unlinked leaves pool repository' do
expect { subject.execute }.to change { project.reload.has_pool_repository? }.from(true).to(false)
end
end
context 'when given project is not part of a fork network' do
let!(:project_without_forks) { create(:project, :public) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment