From 5cd677fa8419c817ef03956b269d9d4ff1d9ba28 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 7 Dec 2020 13:18:34 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@13-4-stable-ee --- CHANGELOG.md | 16 ++++ GITALY_SERVER_VERSION | 2 +- app/controllers/users_controller.rb | 2 +- app/finders/starred_projects_finder.rb | 11 +++ .../security-290-graphql-exposed-email.yml | 5 -- .../security-296-private_profile_exposure.yml | 5 -- .../security-hide-email-in-confirmation-page.yml | 5 -- .../unreleased/security-idor-ff-user-list.yml | 5 -- changelogs/unreleased/security-mermaid-rc-13-6.yml | 5 -- ...-prevent-short-searches-in-explore-projects.yml | 5 -- .../security-project-import-zoom-xss.yml | 5 -- .../unreleased/security-search-term-logged.yml | 5 -- spec/controllers/users_controller_spec.rb | 89 +++++++++++++++++++--- spec/finders/starred_projects_finder_spec.rb | 59 +++++++++++--- .../graphql/user/starred_projects_query_spec.rb | 27 +++++++ spec/requests/api/projects_spec.rb | 45 +++++++++-- vendor/gitignore/C++.gitignore | 0 vendor/gitignore/Java.gitignore | 0 18 files changed, 221 insertions(+), 70 deletions(-) delete mode 100644 changelogs/unreleased/security-290-graphql-exposed-email.yml delete mode 100644 changelogs/unreleased/security-296-private_profile_exposure.yml delete mode 100644 changelogs/unreleased/security-hide-email-in-confirmation-page.yml delete mode 100644 changelogs/unreleased/security-idor-ff-user-list.yml delete mode 100644 changelogs/unreleased/security-mermaid-rc-13-6.yml delete mode 100644 changelogs/unreleased/security-prevent-short-searches-in-explore-projects.yml delete mode 100644 changelogs/unreleased/security-project-import-zoom-xss.yml delete mode 100644 changelogs/unreleased/security-search-term-logged.yml mode change 100644 => 100755 vendor/gitignore/C++.gitignore mode change 100644 => 100755 vendor/gitignore/Java.gitignore diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f5f837d9c3..84f3126d785 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 13.4.7 (2020-12-07) + +### Security (10 changes) + +- Validate zoom links to start with https only. !1055 +- Require at least 3 characters when searching for project in the Explore page. +- Do not show emails of users in confirmation page. +- Forbid setting a gitlabUserList strategy to a list from another project. +- Fix mermaid resource consumption in GFM fields. +- Ensure group and project memberships are not leaked via API for users with private profiles. +- GraphQL User: do not expose email if set to private. +- Filter search parameter to prevent data leaks. +- Do not expose starred projects of users with private profile via API. +- Do not show starred & contributed projects of users with private profile. + + ## 13.4.6 (2020-11-03) ### Fixed (1 change) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 56f3fd065d7..b64e52773f4 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -13.4.6 \ No newline at end of file +13.4.7 \ No newline at end of file diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 75a861423ed..f4757316e1d 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -19,7 +19,7 @@ class UsersController < ApplicationController prepend_before_action(only: [:show]) { authenticate_sessionless_user!(:rss) } before_action :user, except: [:exists, :suggests] before_action :authorize_read_user_profile!, - only: [:calendar, :calendar_activities, :groups, :projects, :contributed_projects, :starred_projects, :snippets] + only: [:calendar, :calendar_activities, :groups, :projects, :contributed, :starred, :snippets] def show respond_to do |format| diff --git a/app/finders/starred_projects_finder.rb b/app/finders/starred_projects_finder.rb index fcb469d1d17..e209960c471 100644 --- a/app/finders/starred_projects_finder.rb +++ b/app/finders/starred_projects_finder.rb @@ -1,11 +1,22 @@ # frozen_string_literal: true class StarredProjectsFinder < ProjectsFinder + include Gitlab::Allowable + def initialize(user, params: {}, current_user: nil) + @user = user + super( params: params, current_user: current_user, project_ids_relation: user.starred_projects.select(:id) ) end + + def execute + # Do not show starred projects if the user has a private profile. + return Project.none unless can?(current_user, :read_user_profile, @user) + + super + end end diff --git a/changelogs/unreleased/security-290-graphql-exposed-email.yml b/changelogs/unreleased/security-290-graphql-exposed-email.yml deleted file mode 100644 index 8b07bb1342f..00000000000 --- a/changelogs/unreleased/security-290-graphql-exposed-email.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: 'GraphQL User: do not expose email if set to private' -merge_request: -author: -type: security diff --git a/changelogs/unreleased/security-296-private_profile_exposure.yml b/changelogs/unreleased/security-296-private_profile_exposure.yml deleted file mode 100644 index 05d98788aed..00000000000 --- a/changelogs/unreleased/security-296-private_profile_exposure.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Ensure group and project memberships are not leaked via API for users with private profiles -merge_request: -author: -type: security diff --git a/changelogs/unreleased/security-hide-email-in-confirmation-page.yml b/changelogs/unreleased/security-hide-email-in-confirmation-page.yml deleted file mode 100644 index b8f448acfcd..00000000000 --- a/changelogs/unreleased/security-hide-email-in-confirmation-page.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Do not show emails of users in confirmation page -merge_request: -author: -type: security diff --git a/changelogs/unreleased/security-idor-ff-user-list.yml b/changelogs/unreleased/security-idor-ff-user-list.yml deleted file mode 100644 index 6d17f9af11d..00000000000 --- a/changelogs/unreleased/security-idor-ff-user-list.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Forbid setting a gitlabUserList strategy to a list from another project -merge_request: -author: -type: security diff --git a/changelogs/unreleased/security-mermaid-rc-13-6.yml b/changelogs/unreleased/security-mermaid-rc-13-6.yml deleted file mode 100644 index 10c620de108..00000000000 --- a/changelogs/unreleased/security-mermaid-rc-13-6.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix mermaid resource consumption in GFM fields -merge_request: -author: -type: security diff --git a/changelogs/unreleased/security-prevent-short-searches-in-explore-projects.yml b/changelogs/unreleased/security-prevent-short-searches-in-explore-projects.yml deleted file mode 100644 index 672ccc09a33..00000000000 --- a/changelogs/unreleased/security-prevent-short-searches-in-explore-projects.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Require at least 3 characters when searching for project in the Explore page -merge_request: -author: -type: security diff --git a/changelogs/unreleased/security-project-import-zoom-xss.yml b/changelogs/unreleased/security-project-import-zoom-xss.yml deleted file mode 100644 index 4f4d7f14b6b..00000000000 --- a/changelogs/unreleased/security-project-import-zoom-xss.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Validate zoom links to start with https only -merge_request: 1055 -author: -type: security diff --git a/changelogs/unreleased/security-search-term-logged.yml b/changelogs/unreleased/security-search-term-logged.yml deleted file mode 100644 index c3e9d1862bd..00000000000 --- a/changelogs/unreleased/security-search-term-logged.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Filter search parameter to prevent data leaks -merge_request: -author: -type: security diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index bec4b24484a..2e57a901319 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -247,32 +247,99 @@ RSpec.describe UsersController do describe 'GET #contributed' do let(:project) { create(:project, :public) } - let(:current_user) { create(:user) } + + subject do + get :contributed, params: { username: author.username }, format: format + end before do - sign_in(current_user) + sign_in(user) project.add_developer(public_user) project.add_developer(private_user) + create(:push_event, project: project, author: author) + + subject end - context 'with public profile' do + shared_examples_for 'renders contributed projects' do it 'renders contributed projects' do - create(:push_event, project: project, author: public_user) + expect(assigns[:contributed_projects]).not_to be_empty + expect(response).to have_gitlab_http_status(:ok) + end + end - get :contributed, params: { username: public_user.username } + %i(html json).each do |format| + context "format: #{format}" do + let(:format) { format } - expect(assigns[:contributed_projects]).not_to be_empty + context 'with public profile' do + let(:author) { public_user } + + it_behaves_like 'renders contributed projects' + end + + context 'with private profile' do + let(:author) { private_user } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'with a user that has the ability to read private profiles', :enable_admin_mode do + let(:user) { create(:admin) } + + it_behaves_like 'renders contributed projects' + end + end + end + end + end + + describe 'GET #starred' do + let(:project) { create(:project, :public) } + + subject do + get :starred, params: { username: author.username }, format: format + end + + before do + author.toggle_star(project) + + sign_in(user) + subject + end + + shared_examples_for 'renders starred projects' do + it 'renders starred projects' do + expect(response).to have_gitlab_http_status(:ok) + expect(assigns[:starred_projects]).not_to be_empty end end - context 'with private profile' do - it 'does not render contributed projects' do - create(:push_event, project: project, author: private_user) + %i(html json).each do |format| + context "format: #{format}" do + let(:format) { format } + + context 'with public profile' do + let(:author) { public_user } + + it_behaves_like 'renders starred projects' + end + + context 'with private profile' do + let(:author) { private_user } + + it 'returns 404' do + expect(response).to have_gitlab_http_status(:not_found) + end - get :contributed, params: { username: private_user.username } + context 'with a user that has the ability to read private profiles', :enable_admin_mode do + let(:user) { create(:admin) } - expect(assigns[:contributed_projects]).to be_empty + it_behaves_like 'renders starred projects' + end + end end end end diff --git a/spec/finders/starred_projects_finder_spec.rb b/spec/finders/starred_projects_finder_spec.rb index 15d4ae52ddd..f5d3314021d 100644 --- a/spec/finders/starred_projects_finder_spec.rb +++ b/spec/finders/starred_projects_finder_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe StarredProjectsFinder do let(:project1) { create(:project, :public, :empty_repo) } let(:project2) { create(:project, :public, :empty_repo) } - let(:other_project) { create(:project, :public, :empty_repo) } + let(:private_project) { create(:project, :private, :empty_repo) } let(:user) { create(:user) } let(:other_user) { create(:user) } @@ -13,6 +13,9 @@ RSpec.describe StarredProjectsFinder do before do user.toggle_star(project1) user.toggle_star(project2) + + private_project.add_maintainer(user) + user.toggle_star(private_project) end describe '#execute' do @@ -20,22 +23,56 @@ RSpec.describe StarredProjectsFinder do subject { finder.execute } - describe 'as same user' do - let(:current_user) { user } + context 'user has a public profile' do + describe 'as same user' do + let(:current_user) { user } - it { is_expected.to contain_exactly(project1, project2) } - end + it { is_expected.to contain_exactly(project1, project2, private_project) } + end + + describe 'as other user' do + let(:current_user) { other_user } - describe 'as other user' do - let(:current_user) { other_user } + it { is_expected.to contain_exactly(project1, project2) } + end - it { is_expected.to contain_exactly(project1, project2) } + describe 'as no user' do + let(:current_user) { nil } + + it { is_expected.to contain_exactly(project1, project2) } + end end - describe 'as no user' do - let(:current_user) { nil } + context 'user has a private profile' do + before do + user.update!(private_profile: true) + end + + describe 'as same user' do + let(:current_user) { user } + + it { is_expected.to contain_exactly(project1, project2, private_project) } + end + + describe 'as other user' do + context 'user does not have access to view the private profile' do + let(:current_user) { other_user } + + it { is_expected.to be_empty } + end + + context 'user has access to view the private profile', :enable_admin_mode do + let(:current_user) { create(:admin) } + + it { is_expected.to contain_exactly(project1, project2, private_project) } + end + end + + describe 'as no user' do + let(:current_user) { nil } - it { is_expected.to contain_exactly(project1, project2) } + it { is_expected.to be_empty } + end end end end diff --git a/spec/requests/api/graphql/user/starred_projects_query_spec.rb b/spec/requests/api/graphql/user/starred_projects_query_spec.rb index 8a1bd3d172f..b098058a735 100644 --- a/spec/requests/api/graphql/user/starred_projects_query_spec.rb +++ b/spec/requests/api/graphql/user/starred_projects_query_spec.rb @@ -70,4 +70,31 @@ RSpec.describe 'Getting starredProjects of the user' do ) end end + + context 'the user has a private profile' do + before do + user.update!(private_profile: true) + post_graphql(query, current_user: current_user) + end + + context 'the current user does not have access to view the private profile of the user' do + let(:current_user) { create(:user) } + + it 'finds no projects' do + expect(starred_projects).to be_empty + end + end + + context 'the current user has access to view the private profile of the user' do + let(:current_user) { create(:admin) } + + it 'finds all projects starred by the user, which the current user has access to' do + expect(starred_projects).to contain_exactly( + a_hash_including('id' => global_id_of(project_a)), + a_hash_including('id' => global_id_of(project_b)), + a_hash_including('id' => global_id_of(project_c)) + ) + end + end + end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 831b0d6e678..205c8655fad 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1255,13 +1255,46 @@ RSpec.describe API::Projects do expect(json_response['message']).to eq('404 User Not Found') end - it 'returns projects filtered by user' do - get api("/users/#{user3.id}/starred_projects/", user) + context 'with a public profile' do + it 'returns projects filtered by user' do + get api("/users/#{user3.id}/starred_projects/", user) - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(json_response.map { |project| project['id'] }).to contain_exactly(project.id, project2.id, project3.id) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }) + .to contain_exactly(project.id, project2.id, project3.id) + end + end + + context 'with a private profile' do + before do + user3.update!(private_profile: true) + user3.reload + end + + context 'user does not have access to view the private profile' do + it 'returns no projects' do + get api("/users/#{user3.id}/starred_projects/", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response).to be_empty + end + end + + context 'user has access to view the private profile' do + it 'returns projects filtered by user' do + get api("/users/#{user3.id}/starred_projects/", admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |project| project['id'] }) + .to contain_exactly(project.id, project2.id, project3.id) + end + end end end diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore old mode 100644 new mode 100755 diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore old mode 100644 new mode 100755 -- cgit v1.2.1