diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-12-03 09:59:41 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-12-03 09:59:41 +0000 |
commit | 6567c4e6e13009456365c76b603893aa7b7595d0 (patch) | |
tree | feafe6c2d8b4cd424fbb936704adb308c22dca03 | |
parent | a26f31a30bd45567e264c56c9e66f9fac721d465 (diff) | |
download | gitlab-ce-6567c4e6e13009456365c76b603893aa7b7595d0.tar.gz |
Add latest changes from gitlab-org/security/gitlab@14-4-stable-ee
-rw-r--r-- | app/assets/javascripts/blob/openapi/index.js | 6 | ||||
-rw-r--r-- | config/application.rb | 1 | ||||
-rw-r--r-- | doc/user/packages/maven_repository/index.md | 2 | ||||
-rw-r--r-- | lib/api/entities/project.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/import_export/members_mapper.rb | 11 | ||||
-rw-r--r-- | lib/gitlab/regex.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/slash_commands/deploy.rb | 12 | ||||
-rw-r--r-- | spec/features/projects/blobs/blob_show_spec.rb | 51 | ||||
-rw-r--r-- | spec/lib/api/entities/project_spec.rb | 22 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/members_mapper_spec.rb | 60 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/project/relation_factory_spec.rb | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/project/tree_restorer_spec.rb | 17 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/regex_spec.rb | 13 | ||||
-rw-r--r-- | spec/lib/gitlab/slash_commands/deploy_spec.rb | 59 | ||||
-rw-r--r-- | spec/models/packages/package_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/projects_spec.rb | 2 |
17 files changed, 255 insertions, 17 deletions
diff --git a/app/assets/javascripts/blob/openapi/index.js b/app/assets/javascripts/blob/openapi/index.js index cb251274b18..b19cc19cb8c 100644 --- a/app/assets/javascripts/blob/openapi/index.js +++ b/app/assets/javascripts/blob/openapi/index.js @@ -1,5 +1,6 @@ import { SwaggerUIBundle } from 'swagger-ui-dist'; import createFlash from '~/flash'; +import { removeParams, updateHistory } from '~/lib/utils/url_utility'; import { __ } from '~/locale'; export default () => { @@ -7,9 +8,14 @@ export default () => { Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')]) .then(() => { + // Temporary fix to prevent an XSS attack due to "useUnsafeMarkdown" + // Once we upgrade Swagger to "4.0.0", we can safely remove this as it will be deprecated + // Follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/339696 + updateHistory({ url: removeParams(['useUnsafeMarkdown']), replace: true }); SwaggerUIBundle({ url: el.dataset.endpoint, dom_id: '#js-openapi-viewer', + useUnsafeMarkdown: false, }); }) .catch((error) => { diff --git a/config/application.rb b/config/application.rb index dba9550a3dc..8bf365e5cdd 100644 --- a/config/application.rb +++ b/config/application.rb @@ -376,6 +376,7 @@ module Gitlab config.cache_store = :redis_cache_store, Gitlab::Redis::Cache.active_support_config config.active_job.queue_adapter = :sidekiq + config.active_job.logger = nil # This is needed for gitlab-shell ENV['GITLAB_PATH_OUTSIDE_HOOK'] = ENV['PATH'] diff --git a/doc/user/packages/maven_repository/index.md b/doc/user/packages/maven_repository/index.md index 17571047353..c1a46a548f4 100644 --- a/doc/user/packages/maven_repository/index.md +++ b/doc/user/packages/maven_repository/index.md @@ -806,7 +806,7 @@ When the pipeline is successful, the package is created. The version string is validated by using the following regex. ```ruby -\A(\.?[\w\+-]+\.?)+\z +\A(?!.*\.\.)[\w+.-]+\z ``` You can play around with the regex and try your version strings on [this regular expression editor](https://rubular.com/r/rrLQqUXjfKEoL6). diff --git a/lib/api/entities/project.rb b/lib/api/entities/project.rb index 41320d184f9..0f19557e6a5 100644 --- a/lib/api/entities/project.rb +++ b/lib/api/entities/project.rb @@ -55,7 +55,9 @@ module API expose(:snippets_enabled) { |project, options| project.feature_available?(:snippets, options[:current_user]) } expose(:container_registry_enabled) { |project, options| project.feature_available?(:container_registry, options[:current_user]) } expose :service_desk_enabled - expose :service_desk_address + expose :service_desk_address, if: -> (project, options) do + Ability.allowed?(options[:current_user], :admin_issue, project) + end expose(:can_create_merge_request_in) do |project, options| Ability.allowed?(options[:current_user], :create_merge_request_in, project) diff --git a/lib/gitlab/import_export/members_mapper.rb b/lib/gitlab/import_export/members_mapper.rb index ce886cb8738..dd7ec361dd8 100644 --- a/lib/gitlab/import_export/members_mapper.rb +++ b/lib/gitlab/import_export/members_mapper.rb @@ -52,11 +52,20 @@ module Gitlab @importable.members.destroy_all # rubocop: disable Cop/DestroyAll - relation_class.create!(user: @user, access_level: highest_access_level, source_id: @importable.id, importing: true) + relation_class.create!(user: @user, access_level: importer_access_level, source_id: @importable.id, importing: true) rescue StandardError => e raise e, "Error adding importer user to #{@importable.class} members. #{e.message}" end + def importer_access_level + if @importable.parent.is_a?(::Group) && !@user.admin? + lvl = @importable.parent.max_member_access_for_user(@user, only_concrete_membership: true) + [lvl, highest_access_level].min + else + highest_access_level + end + end + def user_already_member? member = @importable.members&.first diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 8b2f786a91a..904fc744c6b 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -57,7 +57,7 @@ module Gitlab end def maven_version_regex - @maven_version_regex ||= /\A(\.?[\w\+-]+\.?)+\z/.freeze + @maven_version_regex ||= /\A(?!.*\.\.)[\w+.-]+\z/.freeze end def maven_app_group_regex diff --git a/lib/gitlab/slash_commands/deploy.rb b/lib/gitlab/slash_commands/deploy.rb index 157d924f99f..9fcefd99f81 100644 --- a/lib/gitlab/slash_commands/deploy.rb +++ b/lib/gitlab/slash_commands/deploy.rb @@ -3,8 +3,18 @@ module Gitlab module SlashCommands class Deploy < BaseCommand + DEPLOY_REGEX = /\Adeploy\s/.freeze + def self.match(text) - /\Adeploy\s+(?<from>\S+.*)\s+to+\s+(?<to>\S+.*)\z/.match(text) + return unless text&.match?(DEPLOY_REGEX) + + from, _, to = text.sub(DEPLOY_REGEX, '').rpartition(/\sto+\s/) + return if from.blank? || to.blank? + + { + from: from.strip, + to: to.strip + } end def self.help_message diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 8281e82959b..9d05c985af1 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -7,8 +7,8 @@ RSpec.describe 'File blob', :js do let(:project) { create(:project, :public, :repository) } - def visit_blob(path, anchor: nil, ref: 'master') - visit project_blob_path(project, File.join(ref, path), anchor: anchor) + def visit_blob(path, anchor: nil, ref: 'master', **additional_args) + visit project_blob_path(project, File.join(ref, path), anchor: anchor, **additional_args) wait_for_requests end @@ -1501,6 +1501,53 @@ RSpec.describe 'File blob', :js do end end end + + context 'openapi.yml' do + before do + file_name = 'openapi.yml' + + create_file(file_name, ' + swagger: \'2.0\' + info: + title: Classic API Resource Documentation + description: | + <div class="foo-bar" style="background-color: red;" data-foo-bar="baz"> + <h1>Swagger API documentation</h1> + </div> + version: production + basePath: /JSSResource/ + produces: + - application/xml + - application/json + consumes: + - application/xml + - application/json + security: + - basicAuth: [] + paths: + /accounts: + get: + responses: + \'200\': + description: No response was specified + tags: + - accounts + operationId: findAccounts + summary: Finds all accounts + ') + visit_blob(file_name, useUnsafeMarkdown: '1') + click_button('Display rendered file') + + wait_for_requests + end + + it 'removes `style`, `class`, and `data-*`` attributes from HTML' do + expect(page).to have_css('h1', text: 'Swagger API documentation') + expect(page).not_to have_css('.foo-bar') + expect(page).not_to have_css('[style="background-color: red;"]') + expect(page).not_to have_css('[data-foo-bar="baz"]') + end + end end end diff --git a/spec/lib/api/entities/project_spec.rb b/spec/lib/api/entities/project_spec.rb index 8d1c3aa878d..6b542278fa6 100644 --- a/spec/lib/api/entities/project_spec.rb +++ b/spec/lib/api/entities/project_spec.rb @@ -13,6 +13,28 @@ RSpec.describe ::API::Entities::Project do subject(:json) { entity.as_json } + describe '.service_desk_address' do + before do + allow(project).to receive(:service_desk_enabled?).and_return(true) + end + + context 'when a user can admin issues' do + before do + project.add_reporter(current_user) + end + + it 'is present' do + expect(json[:service_desk_address]).to be_present + end + end + + context 'when a user can not admin project' do + it 'is empty' do + expect(json[:service_desk_address]).to be_nil + end + end + end + describe '.shared_with_groups' do let(:group) { create(:group, :private) } diff --git a/spec/lib/gitlab/import_export/members_mapper_spec.rb b/spec/lib/gitlab/import_export/members_mapper_spec.rb index 847d6b5d1ed..8b9ca90a280 100644 --- a/spec/lib/gitlab/import_export/members_mapper_spec.rb +++ b/spec/lib/gitlab/import_export/members_mapper_spec.rb @@ -267,6 +267,66 @@ RSpec.describe Gitlab::ImportExport::MembersMapper do end end + context 'when importer is not an admin' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:members_mapper) do + described_class.new( + exported_members: [], user: user, importable: importable) + end + + shared_examples_for 'it fetches the access level from parent group' do + before do + group.add_users([user], group_access_level) + end + + it "and resolves it correctly" do + members_mapper.map + expect(member_class.find_by_user_id(user.id).access_level).to eq(resolved_access_level) + end + end + + context 'and the imported project is part of a group' do + let(:importable) { create(:project, namespace: group) } + let(:member_class) { ProjectMember } + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::DEVELOPER } + let(:resolved_access_level) { ProjectMember::DEVELOPER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::MAINTAINER } + let(:resolved_access_level) { ProjectMember::MAINTAINER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::OWNER } + let(:resolved_access_level) { ProjectMember::MAINTAINER } + end + end + + context 'and the imported group is part of another group' do + let(:importable) { create(:group, parent: group) } + let(:member_class) { GroupMember } + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::DEVELOPER } + let(:resolved_access_level) { GroupMember::DEVELOPER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::MAINTAINER } + let(:resolved_access_level) { GroupMember::MAINTAINER } + end + + it_behaves_like 'it fetches the access level from parent group' do + let(:group_access_level) { GroupMember::OWNER } + let(:resolved_access_level) { GroupMember::OWNER } + end + end + end + context 'when importable is Group' do include_examples 'imports exported members' do let(:source_type) { 'Namespace' } diff --git a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb index 49df2313924..80ba50976af 100644 --- a/spec/lib/gitlab/import_export/project/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/project/relation_factory_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe Gitlab::ImportExport::Project::RelationFactory, :use_clean_rails_memory_store_caching do - let(:group) { create(:group) } + let(:group) { create(:group).tap { |g| g.add_maintainer(importer_user) } } let(:project) { create(:project, :repository, group: group) } let(:members_mapper) { double('members_mapper').as_null_object } let(:admin) { create(:admin) } diff --git a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb index f512f49764d..79cf20fbaca 100644 --- a/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project/tree_restorer_spec.rb @@ -675,6 +675,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do # Project needs to be in a group for visibility level comparison # to happen group = create(:group) + group.add_maintainer(user) project.group = group project.create_import_data(data: { override_params: { visibility_level: Gitlab::VisibilityLevel::INTERNAL.to_s } }) @@ -716,13 +717,19 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end context 'with a project that has a group' do + let(:group) do + create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE).tap do |g| + g.add_maintainer(user) + end + end + let!(:project) do create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project', - group: create(:group, visibility_level: Gitlab::VisibilityLevel::PRIVATE)) + group: group) end before do @@ -751,13 +758,14 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end context 'with existing group models' do + let(:group) { create(:group).tap { |g| g.add_maintainer(user) } } let!(:project) do create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project', - group: create(:group)) + group: group) end before do @@ -786,13 +794,14 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do end context 'with clashing milestones on IID' do + let(:group) { create(:group).tap { |g| g.add_maintainer(user) } } let!(:project) do create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project', - group: create(:group)) + group: group) end before do @@ -871,7 +880,7 @@ RSpec.describe Gitlab::ImportExport::Project::TreeRestorer do context 'with group visibility' do before do group = create(:group, visibility_level: group_visibility) - + group.add_users([user], GroupMember::MAINTAINER) project.update(group: group) end diff --git a/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb index 5e4075c2b59..41d4c1f0b9e 100644 --- a/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/relation_tree_restorer_spec.rb @@ -105,7 +105,7 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do it_behaves_like 'import project successfully' context 'logging of relations creation' do - let_it_be(:group) { create(:group) } + let_it_be(:group) { create(:group).tap { |g| g.add_maintainer(user) } } let_it_be(:importable) do create(:project, :builds_enabled, :issues_disabled, name: 'project', path: 'project', group: group) end @@ -122,7 +122,7 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do context 'when inside a group' do let_it_be(:group) do - create(:group, :disabled_and_unoverridable) + create(:group, :disabled_and_unoverridable).tap { |g| g.add_maintainer(user) } end before do @@ -155,7 +155,7 @@ RSpec.describe Gitlab::ImportExport::RelationTreeRestorer do context 'when restoring a group' do let_it_be(:group) { create(:group) } - let_it_be(:importable) { create(:group, parent: group) } + let_it_be(:importable) { create(:group, parent: group).tap { |g| g.add_owner(user) } } let(:path) { 'spec/fixtures/lib/gitlab/import_export/group_exports/no_children/group.json' } let(:importable_name) { nil } diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 9514654204b..05f1c88a6ab 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -344,6 +344,18 @@ RSpec.describe Gitlab::Regex do describe '.maven_version_regex' do subject { described_class.maven_version_regex } + it 'has no ReDoS issues with long strings' do + Timeout.timeout(5) do + expect(subject).to match("aaaaaaaa.aaaaaaaaa+aa-111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111.11111111111111111111111111111111111111111111111111111111") + end + end + + it 'has no ReDos issues with long strings ending with an exclamation mark' do + Timeout.timeout(5) do + expect(subject).not_to match('a' * 50000 + '!') + end + end + it { is_expected.to match('0')} it { is_expected.to match('1') } it { is_expected.to match('03') } @@ -364,6 +376,7 @@ RSpec.describe Gitlab::Regex do it { is_expected.to match('703220b4e2cea9592caeb9f3013f6b1e5335c293') } it { is_expected.to match('RELEASE') } it { is_expected.not_to match('..1.2.3') } + it { is_expected.not_to match('1.2.3..beta') } it { is_expected.not_to match(' 1.2.3') } it { is_expected.not_to match("1.2.3 \r\t") } it { is_expected.not_to match("\r\t 1.2.3") } diff --git a/spec/lib/gitlab/slash_commands/deploy_spec.rb b/spec/lib/gitlab/slash_commands/deploy_spec.rb index 36f47c711bc..71fca1e1fc8 100644 --- a/spec/lib/gitlab/slash_commands/deploy_spec.rb +++ b/spec/lib/gitlab/slash_commands/deploy_spec.rb @@ -109,6 +109,21 @@ RSpec.describe Gitlab::SlashCommands::Deploy do end end end + + context 'with extra spaces in the deploy command' do + let(:regex_match) { described_class.match('deploy staging to production ') } + + before do + create(:ci_build, :manual, pipeline: pipeline, name: 'production', environment: 'production') + create(:ci_build, :manual, pipeline: pipeline, name: 'not prod', environment: 'not prod') + end + + it 'deploys to production' do + expect(subject[:text]) + .to start_with('Deployment started from staging to production') + expect(subject[:response_type]).to be(:in_channel) + end + end end end @@ -119,5 +134,49 @@ RSpec.describe Gitlab::SlashCommands::Deploy do expect(match[:from]).to eq('staging') expect(match[:to]).to eq('production') end + + it 'matches the environment with spaces in it' do + match = described_class.match('deploy staging env to production env') + + expect(match[:from]).to eq('staging env') + expect(match[:to]).to eq('production env') + end + + it 'matches the environment name with surrounding spaces' do + match = described_class.match('deploy staging to production ') + + # The extra spaces are stripped later in the code + expect(match[:from]).to eq('staging') + expect(match[:to]).to eq('production') + end + + it 'returns nil for text that is not a deploy command' do + match = described_class.match('foo bar') + + expect(match).to be_nil + end + + it 'returns nil for a partial command' do + match = described_class.match('deploy staging to ') + + expect(match).to be_nil + end + + context 'with ReDoS attempts' do + def duration_for(&block) + start = Time.zone.now + yield if block_given? + Time.zone.now - start + end + + it 'has smaller than linear execution time growth with a malformed "to"' do + Timeout.timeout(3.seconds) do + sample1 = duration_for { described_class.match("deploy abc t" + "o" * 1000 + "X") } + sample2 = duration_for { described_class.match("deploy abc t" + "o" * 4000 + "X") } + + expect((sample2 / sample1) < 4).to be_truthy + end + end + end end end diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 2573c01d686..996fe0757e8 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -289,7 +289,6 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to allow_value('1.1-beta-2').for(:version) } it { is_expected.to allow_value('1.2-SNAPSHOT').for(:version) } it { is_expected.to allow_value('12.1.2-2-1').for(:version) } - it { is_expected.to allow_value('1.2.3..beta').for(:version) } it { is_expected.to allow_value('1.2.3-beta').for(:version) } it { is_expected.to allow_value('10.2.3-beta').for(:version) } it { is_expected.to allow_value('2.0.0.v200706041905-7C78EK9E_EkMNfNOd2d8qq').for(:version) } @@ -297,6 +296,7 @@ RSpec.describe Packages::Package, type: :model do it { is_expected.to allow_value('703220b4e2cea9592caeb9f3013f6b1e5335c293').for(:version) } it { is_expected.to allow_value('RELEASE').for(:version) } it { is_expected.not_to allow_value('..1.2.3').for(:version) } + it { is_expected.not_to allow_value('1.2.3..beta').for(:version) } it { is_expected.not_to allow_value(' 1.2.3').for(:version) } it { is_expected.not_to allow_value("1.2.3 \r\t").for(:version) } it { is_expected.not_to allow_value("\r\t 1.2.3").for(:version) } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index dd6afa869e0..1c79093a831 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -224,7 +224,7 @@ RSpec.describe API::Projects do create(:project, :public, group: create(:group)) end - it_behaves_like 'projects response without N + 1 queries', 0 do + it_behaves_like 'projects response without N + 1 queries', 1 do let(:current_user) { user } let(:additional_project) { create(:project, :public, group: create(:group)) } end |