New upstream version 13.6.6

This commit is contained in:
Pirate Praveen 2021-02-04 15:43:07 +05:30
parent f48d5478cb
commit 3571a16ade
18 changed files with 203 additions and 9 deletions

View file

@ -2,6 +2,17 @@
documentation](doc/development/changelog.md) for instructions on adding your own documentation](doc/development/changelog.md) for instructions on adding your own
entry. entry.
## 13.6.6 (2021-02-01)
### Security (5 changes)
- Filter sensitive GraphQL variables from logs.
- Avoid exposing release links when the user cannot read git-tag/repository.
- Sanitize target branch on MR page.
- Fix DNS rebinding protection bypass when allowing an IP address in Outbound Requests setting.
- Add routes for unmatched url for not-get requests.
## 13.6.5 (2021-01-13) ## 13.6.5 (2021-01-13)
### Security (1 change) ### Security (1 change)

View file

@ -1 +1 @@
13.6.5 13.6.6

View file

@ -1 +1 @@
13.6.5 13.6.6

View file

@ -1,5 +1,6 @@
<script> <script>
import { isNumber } from 'lodash'; import { isNumber } from 'lodash';
import { sanitize } from '~/lib/dompurify';
import ArtifactsApp from './artifacts_list_app.vue'; import ArtifactsApp from './artifacts_list_app.vue';
import Deployment from './deployment/deployment.vue'; import Deployment from './deployment/deployment.vue';
import MrWidgetContainer from './mr_widget_container.vue'; import MrWidgetContainer from './mr_widget_container.vue';
@ -41,7 +42,7 @@ export default {
return this.isPostMerge ? this.mr.targetBranch : this.mr.sourceBranch; return this.isPostMerge ? this.mr.targetBranch : this.mr.sourceBranch;
}, },
branchLink() { branchLink() {
return this.isPostMerge ? this.mr.targetBranch : this.mr.sourceBranchLink; return this.isPostMerge ? sanitize(this.mr.targetBranch) : this.mr.sourceBranchLink;
}, },
deployments() { deployments() {
return this.isPostMerge ? this.mr.postMergeDeployments : this.mr.deployments; return this.isPostMerge ? this.mr.postMergeDeployments : this.mr.deployments;

View file

@ -5,6 +5,9 @@ class Projects::ReleasesController < Projects::ApplicationController
before_action :require_non_empty_project, except: [:index] before_action :require_non_empty_project, except: [:index]
before_action :release, only: %i[edit show update downloads] before_action :release, only: %i[edit show update downloads]
before_action :authorize_read_release! before_action :authorize_read_release!
# We have to check `download_code` permission because detail URL path
# contains git-tag name.
before_action :authorize_download_code!, except: [:index]
before_action do before_action do
push_frontend_feature_flag(:graphql_release_data, project, default_enabled: true) push_frontend_feature_flag(:graphql_release_data, project, default_enabled: true)
push_frontend_feature_flag(:graphql_milestone_stats, project, default_enabled: true) push_frontend_feature_flag(:graphql_milestone_stats, project, default_enabled: true)

View file

@ -20,6 +20,8 @@ class ReleasePresenter < Gitlab::View::Presenter::Delegated
end end
def self_url def self_url
return unless can_download_code?
project_release_url(project, release) project_release_url(project, release)
end end

View file

@ -267,6 +267,7 @@ Rails.application.routes.draw do
draw :dashboard draw :dashboard
draw :user draw :user
draw :project draw :project
draw :unmatched_project
# Issue https://gitlab.com/gitlab-org/gitlab/-/issues/210024 # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/210024
scope as: 'deprecated' do scope as: 'deprecated' do

View file

@ -0,0 +1,18 @@
# frozen_string_literal: true
scope(path: '*namespace_id',
as: :namespace,
namespace_id: Gitlab::PathRegex.full_namespace_route_regex) do
scope(path: ':project_id',
constraints: { project_id: Gitlab::PathRegex.project_route_regex },
as: :project) do
post '*all', to: 'application#route_not_found'
put '*all', to: 'application#route_not_found'
patch '*all', to: 'application#route_not_found'
delete '*all', to: 'application#route_not_found'
post '/', to: 'application#route_not_found'
put '/', to: 'application#route_not_found'
patch '/', to: 'application#route_not_found'
delete '/', to: 'application#route_not_found'
end
end

View file

@ -49,13 +49,21 @@ module Gitlab
private private
def process_variables(variables) def process_variables(variables)
if variables.respond_to?(:to_s) filtered_variables = filter_sensitive_variables(variables)
variables.to_s
if filtered_variables.respond_to?(:to_s)
filtered_variables.to_s
else else
variables filtered_variables
end end
end end
def filter_sensitive_variables(variables)
ActiveSupport::ParameterFilter
.new(::Rails.application.config.filter_parameters)
.filter(variables)
end
def duration(time_started) def duration(time_started)
Gitlab::Metrics::System.monotonic_time - time_started Gitlab::Metrics::System.monotonic_time - time_started
end end

View file

@ -49,10 +49,12 @@ module Gitlab
return [uri, nil] unless address_info return [uri, nil] unless address_info
ip_address = ip_address(address_info) ip_address = ip_address(address_info)
return [uri, nil] if domain_allowed?(uri) || ip_allowed?(ip_address, port: get_port(uri)) return [uri, nil] if domain_allowed?(uri)
protected_uri_with_hostname = enforce_uri_hostname(ip_address, uri, dns_rebind_protection) protected_uri_with_hostname = enforce_uri_hostname(ip_address, uri, dns_rebind_protection)
return protected_uri_with_hostname if ip_allowed?(ip_address, port: get_port(uri))
# Allow url from the GitLab instance itself but only for the configured hostname and ports # Allow url from the GitLab instance itself but only for the configured hostname and ports
return protected_uri_with_hostname if internal?(uri) return protected_uri_with_hostname if internal?(uri)

View file

@ -9,6 +9,7 @@ RSpec.describe Projects::ReleasesController do
let_it_be(:private_project) { create(:project, :repository, :private) } let_it_be(:private_project) { create(:project, :repository, :private) }
let_it_be(:developer) { create(:user) } let_it_be(:developer) { create(:user) }
let_it_be(:reporter) { create(:user) } let_it_be(:reporter) { create(:user) }
let_it_be(:guest) { create(:user) }
let_it_be(:user) { developer } let_it_be(:user) { developer }
let!(:release_1) { create(:release, project: project, released_at: Time.zone.parse('2018-10-18')) } let!(:release_1) { create(:release, project: project, released_at: Time.zone.parse('2018-10-18')) }
let!(:release_2) { create(:release, project: project, released_at: Time.zone.parse('2019-10-19')) } let!(:release_2) { create(:release, project: project, released_at: Time.zone.parse('2019-10-19')) }
@ -16,6 +17,7 @@ RSpec.describe Projects::ReleasesController do
before do before do
project.add_developer(developer) project.add_developer(developer)
project.add_reporter(reporter) project.add_reporter(reporter)
project.add_guest(guest)
end end
shared_examples_for 'successful request' do shared_examples_for 'successful request' do
@ -199,6 +201,13 @@ RSpec.describe Projects::ReleasesController do
it_behaves_like 'not found' it_behaves_like 'not found'
end end
context 'when user is a guest' do
let(:project) { private_project }
let(:user) { guest }
it_behaves_like 'not found'
end
end end
# `GET #downloads` is addressed in spec/requests/projects/releases_controller_spec.rb # `GET #downloads` is addressed in spec/requests/projects/releases_controller_spec.rb

View file

@ -78,6 +78,18 @@ describe('MrWidgetPipelineContainer', () => {
}); });
}); });
it('sanitizes the targetBranch', () => {
factory({
isPostMerge: true,
mr: {
...mockStore,
targetBranch: 'Foo<script>alert("XSS")</script>',
},
});
expect(wrapper.find(MrWidgetPipeline).props().sourceBranchLink).toBe('Foo');
});
it('renders deployments', () => { it('renders deployments', () => {
const expectedProps = mockStore.postMergeDeployments.map(dep => const expectedProps = mockStore.postMergeDeployments.map(dep =>
expect.objectContaining({ expect.objectContaining({

View file

@ -40,4 +40,22 @@ RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do
end end
end end
end end
describe '#initial_value' do
it 'filters out sensitive variables' do
doc = GraphQL.parse <<-GRAPHQL
mutation createNote($body: String!) {
createNote(input: {noteableId: "1", body: $body}) {
note {
id
}
}
}
GRAPHQL
query = GraphQL::Query.new(GitlabSchema, document: doc, context: {}, variables: { body: "some note" })
expect(subject.initial_value(query)[:variables]).to eq('{:body=>"[FILTERED]"}')
end
end
end end

View file

@ -91,6 +91,21 @@ RSpec.describe Gitlab::UrlBlocker, :stub_invalid_dns_only do
end end
end end
context 'DNS rebinding protection with IP allowed' do
let(:import_url) { 'http://a.192.168.0.120.3times.127.0.0.1.1time.repeat.rebind.network:9121/scrape?target=unix:///var/opt/gitlab/redis/redis.socket&amp;check-keys=*' }
before do
stub_dns(import_url, ip_address: '192.168.0.120')
allow(Gitlab::UrlBlockers::UrlAllowlist).to receive(:ip_allowed?).and_return(true)
end
it_behaves_like 'validates URI and hostname' do
let(:expected_uri) { 'http://192.168.0.120:9121/scrape?target=unix:///var/opt/gitlab/redis/redis.socket&amp;check-keys=*' }
let(:expected_hostname) { 'a.192.168.0.120.3times.127.0.0.1.1time.repeat.rebind.network' }
end
end
context 'disabled DNS rebinding protection' do context 'disabled DNS rebinding protection' do
subject { described_class.validate!(import_url, dns_rebind_protection: false) } subject { described_class.validate!(import_url, dns_rebind_protection: false) }

View file

@ -62,6 +62,12 @@ RSpec.describe ReleasePresenter do
it 'returns its own url' do it 'returns its own url' do
is_expected.to eq(project_release_url(project, release)) is_expected.to eq(project_release_url(project, release))
end end
context 'when user is guest' do
let(:user) { guest }
it { is_expected.to be_nil }
end
end end
describe '#opened_merge_requests_url' do describe '#opened_merge_requests_url' do

View file

@ -159,13 +159,17 @@ RSpec.describe 'Git HTTP requests' do
context "POST git-upload-pack" do context "POST git-upload-pack" do
it "fails to find a route" do it "fails to find a route" do
expect { clone_post(repository_path) }.to raise_error(ActionController::RoutingError) clone_post(repository_path) do |response|
expect(response).to have_gitlab_http_status(:not_found)
end
end end
end end
context "POST git-receive-pack" do context "POST git-receive-pack" do
it "fails to find a route" do it "fails to find a route" do
expect { push_post(repository_path) }.to raise_error(ActionController::RoutingError) push_post(repository_path) do |response|
expect(response).to have_gitlab_http_status(:not_found)
end
end end
end end
end end

View file

@ -876,4 +876,73 @@ RSpec.describe 'project routing' do
) )
end end
end end
context 'with a non-existent project' do
it 'routes to 404 with get request' do
expect(get: "/gitlab/not_exist").to route_to(
'application#route_not_found',
unmatched_route: 'gitlab/not_exist'
)
end
it 'routes to 404 with delete request' do
expect(delete: "/gitlab/not_exist").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist'
)
end
it 'routes to 404 with post request' do
expect(post: "/gitlab/not_exist").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist'
)
end
it 'routes to 404 with put request' do
expect(put: "/gitlab/not_exist").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist'
)
end
context 'with route to some action' do
it 'routes to 404 with get request to' do
expect(get: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
unmatched_route: 'gitlab/not_exist/some_action'
)
end
it 'routes to 404 with delete request' do
expect(delete: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist',
all: 'some_action'
)
end
it 'routes to 404 with post request' do
expect(post: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist',
all: 'some_action'
)
end
it 'routes to 404 with put request' do
expect(put: "/gitlab/not_exist/some_action").to route_to(
'application#route_not_found',
namespace_id: 'gitlab',
project_id: 'not_exist',
all: 'some_action'
)
end
end
end
end end

View file

@ -0,0 +1,15 @@
# frozen_string_literal: true
RSpec::Matchers.define :route_to_route_not_found do
match do |actual|
expect(actual).to route_to(controller: 'application', action: 'route_not_found')
rescue RSpec::Expectations::ExpectationNotMetError => e
# `route_to` matcher requires providing all params for exact match. As we use it in shared examples and we provide different paths,
# this matcher checks if provided route matches controller and action, without checking params.
expect(e.message).to include("-{\"controller\"=>\"application\", \"action\"=>\"route_not_found\"}\n+{\"controller\"=>\"application\", \"action\"=>\"route_not_found\",")
end
failure_message do |_|
"expected #{actual} to route to route_not_found"
end
end