diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-30 15:16:56 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-10-30 15:16:56 +0000 |
commit | fa2fec1d18330e4cd9803ff164db19e7367e3838 (patch) | |
tree | 91a9bf1c74eeff29690f33e3faf2b8ca87051af3 | |
parent | 8ee0746f54c19fcb8fe81058594aa8d373c5b7d7 (diff) | |
download | gitlab-ce-fa2fec1d18330e4cd9803ff164db19e7367e3838.tar.gz |
Add latest changes from gitlab-org/security/gitlab@13-5-stable-ee
22 files changed, 317 insertions, 22 deletions
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 013861631a1..5743efab81b 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -79,11 +79,7 @@ module Projects # Directories on disk move_project_folders(project) - # Move missing group labels to project - Labels::TransferService.new(current_user, @old_group, project).execute - - # Move missing group milestones - Milestones::TransferService.new(current_user, @old_group, project).execute + transfer_missing_group_resources(@old_group) # Move uploads move_project_uploads(project) @@ -107,6 +103,12 @@ module Projects refresh_permissions end + def transfer_missing_group_resources(group) + Labels::TransferService.new(current_user, group, project).execute + + Milestones::TransferService.new(current_user, group, project).execute + end + def allowed_transfer?(current_user, project) @new_namespace && can?(current_user, :change_namespace, project) && diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index 654bb15378c..411d8b2614f 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -5,6 +5,10 @@ class GitlabUploader < CarrierWave::Uploader::Base class_attribute :options + PROTECTED_METHODS = %i(filename cache_dir work_dir store_dir).freeze + + ObjectNotReadyError = Class.new(StandardError) + class << self # DSL setter def storage_options(options) @@ -33,6 +37,8 @@ class GitlabUploader < CarrierWave::Uploader::Base delegate :base_dir, :file_storage?, to: :class + before :cache, :protect_from_path_traversal! + def initialize(model, mounted_as = nil, **uploader_context) super(model, mounted_as) end @@ -121,6 +127,9 @@ class GitlabUploader < CarrierWave::Uploader::Base # For example, `FileUploader` builds the storage path based on the associated # project model's `path_with_namespace` value, which can change when the # project or its containing namespace is moved or renamed. + # + # When implementing this method, raise `ObjectNotReadyError` if the model + # does not yet exist, as it will be tested in `#protect_from_path_traversal!` def dynamic_segment raise(NotImplementedError) end @@ -138,4 +147,21 @@ class GitlabUploader < CarrierWave::Uploader::Base def pathname @pathname ||= Pathname.new(path) end + + # Protect against path traversal attacks + # This takes a list of methods to test for path traversal, e.g. ../../ + # and checks each of them. This uses `.send` so that any potential errors + # don't block the entire set from being tested. + # + # @param [CarrierWave::SanitizedFile] + # @return [Nil] + # @raise [Gitlab::Utils::PathTraversalAttackError] + def protect_from_path_traversal!(file) + PROTECTED_METHODS.each do |method| + Gitlab::Utils.check_path_traversal!(self.send(method)) # rubocop: disable GitlabSecurity/PublicSend + + rescue ObjectNotReadyError + # Do nothing. This test was attempted before the file was ready for that method + end + end end diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb index 47976c909e8..83dc1030606 100644 --- a/app/uploaders/job_artifact_uploader.rb +++ b/app/uploaders/job_artifact_uploader.rb @@ -4,7 +4,6 @@ class JobArtifactUploader < GitlabUploader extend Workhorse::UploadPath include ObjectStorage::Concern - ObjectNotReadyError = Class.new(StandardError) UnknownFileLocationError = Class.new(StandardError) storage_options Gitlab.config.artifacts @@ -24,7 +23,9 @@ class JobArtifactUploader < GitlabUploader private def dynamic_segment - raise ObjectNotReadyError, 'JobArtifact is not ready' unless model.id + # This now tests model.created_at because it can for some reason be nil in the test suite, + # and it's not clear if this is intentional or not + raise ObjectNotReadyError, 'JobArtifact is not ready' unless model.id && model.created_at if model.hashed_path? hashed_path diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb index 28545b9fcdf..61fbe2b4c49 100644 --- a/app/uploaders/packages/package_file_uploader.rb +++ b/app/uploaders/packages/package_file_uploader.rb @@ -20,6 +20,8 @@ class Packages::PackageFileUploader < GitlabUploader private def dynamic_segment + raise ObjectNotReadyError, "Package model not ready" unless model.id + Gitlab::HashedPath.new('packages', model.package.id, 'files', model.id, root_hash: model.package.project_id) end end diff --git a/changelogs/unreleased/security-10io-multipart-check-brackets-balance-in-param-names.yml b/changelogs/unreleased/security-10io-multipart-check-brackets-balance-in-param-names.yml new file mode 100644 index 00000000000..e0d1a4e535d --- /dev/null +++ b/changelogs/unreleased/security-10io-multipart-check-brackets-balance-in-param-names.yml @@ -0,0 +1,5 @@ +--- +title: Validate each upload param key in multipart.rb +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-255886.yml b/changelogs/unreleased/security-255886.yml new file mode 100644 index 00000000000..8fe8da59444 --- /dev/null +++ b/changelogs/unreleased/security-255886.yml @@ -0,0 +1,5 @@ +--- +title: Path traversal to RCE via LFS upload +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-container-regex-backtrack.yml b/changelogs/unreleased/security-container-regex-backtrack.yml new file mode 100644 index 00000000000..c88fd526d47 --- /dev/null +++ b/changelogs/unreleased/security-container-regex-backtrack.yml @@ -0,0 +1,5 @@ +--- +title: Update container_repository_name_regex to prevent catastrophic backtracking +merge_request: +author: +type: security diff --git a/changelogs/unreleased/security-kubernetes-agent-internal-api.yml b/changelogs/unreleased/security-kubernetes-agent-internal-api.yml new file mode 100644 index 00000000000..9ed192e90cc --- /dev/null +++ b/changelogs/unreleased/security-kubernetes-agent-internal-api.yml @@ -0,0 +1,5 @@ +--- +title: Prevent private repo from being accessed via internal Kubernetes API +merge_request: +author: +type: security diff --git a/lib/api/internal/kubernetes.rb b/lib/api/internal/kubernetes.rb index 8175b81f900..90e224b2ccb 100644 --- a/lib/api/internal/kubernetes.rb +++ b/lib/api/internal/kubernetes.rb @@ -85,7 +85,7 @@ module API # TODO sort out authorization for real # https://gitlab.com/gitlab-org/gitlab/-/issues/220912 - if !project || !project.public? + unless Ability.allowed?(nil, :download_code, project) not_found! end diff --git a/lib/gitlab/middleware/multipart.rb b/lib/gitlab/middleware/multipart.rb index 7e98f1fc1f7..a6d8a778e05 100644 --- a/lib/gitlab/middleware/multipart.rb +++ b/lib/gitlab/middleware/multipart.rb @@ -31,6 +31,7 @@ module Gitlab RACK_ENV_KEY = 'HTTP_GITLAB_WORKHORSE_MULTIPART_FIELDS' JWT_PARAM_SUFFIX = '.gitlab-workhorse-upload' JWT_PARAM_FIXED_KEY = 'upload' + REWRITTEN_FIELD_NAME_MAX_LENGTH = 10000.freeze class Handler def initialize(env, message) @@ -41,6 +42,8 @@ module Gitlab def with_open_files @rewritten_fields.each do |field, tmp_path| + raise "invalid field: #{field.inspect}" unless valid_field_name?(field) + parsed_field = Rack::Utils.parse_nested_query(field) raise "unexpected field: #{field.inspect}" unless parsed_field.count == 1 @@ -108,6 +111,17 @@ module Gitlab private + def valid_field_name?(name) + # length validation + return false if name.size >= REWRITTEN_FIELD_NAME_MAX_LENGTH + + # brackets validation + return false if name.include?('[]') || name.start_with?('[', ']') + return false unless ::Gitlab::Utils.valid_brackets?(name, allow_nested: false) + + true + end + def package_allowed_paths packages_config = ::Gitlab.config.packages return [] unless allow_packages_storage_path?(packages_config) @@ -141,6 +155,8 @@ module Gitlab class HandlerForJWTParams < Handler def with_open_files @rewritten_fields.keys.each do |field| + raise "invalid field: #{field.inspect}" unless valid_field_name?(field) + parsed_field = Rack::Utils.parse_nested_query(field) raise "unexpected field: #{field.inspect}" unless parsed_field.count == 1 diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index 693a10a9de3..8dae77f68f3 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -204,7 +204,7 @@ module Gitlab # See https://github.com/docker/distribution/blob/master/reference/regexp.go. # def container_repository_name_regex - @container_repository_regex ||= %r{\A[a-z0-9]+((?:[._/]|__|[-]{0,10})[a-z0-9]+)*\Z} + @container_repository_regex ||= %r{\A[a-z0-9]+(([._/]|__|-*)[a-z0-9])*\z} end ## diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index e2d93e7cd29..3df54e74b4f 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -10,6 +10,8 @@ module Gitlab # Also see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24223#note_284122580 # It also checks for ALT_SEPARATOR aka '\' (forward slash) def check_path_traversal!(path) + return unless path.is_a?(String) + path = decode_path(path) path_regex = /(\A(\.{1,2})\z|\A\.\.[\/\\]|[\/\\]\.\.\z|[\/\\]\.\.[\/\\]|\n)/ @@ -208,5 +210,33 @@ module Gitlab def stable_sort_by(list) list.sort_by.with_index { |x, idx| [yield(x), idx] } end + + # Check for valid brackets (`[` and `]`) in a string using this aspects: + # * open brackets count == closed brackets count + # * (optionally) reject nested brackets via `allow_nested: false` + # * open / close brackets coherence, eg. ][[] -> invalid + def valid_brackets?(string = '', allow_nested: true) + # remove everything except brackets + brackets = string.remove(/[^\[\]]/) + + return true if brackets.empty? + # balanced counts check + return false if brackets.size.odd? + + unless allow_nested + # nested brackets check + return false if brackets.include?('[[') || brackets.include?(']]') + end + + # open / close brackets coherence check + untrimmed = brackets + loop do + trimmed = untrimmed.gsub('[]', '') + return true if trimmed.empty? + return false if trimmed == untrimmed + + untrimmed = trimmed + end + end end end diff --git a/spec/features/file_uploads/multipart_invalid_uploads_spec.rb b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb new file mode 100644 index 00000000000..e9e24c12af1 --- /dev/null +++ b/spec/features/file_uploads/multipart_invalid_uploads_spec.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Invalid uploads that must be rejected', :api, :js do + include_context 'file upload requests helpers' + + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user, :admin) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + + context 'invalid upload key', :capybara_ignore_server_errors do + let(:api_path) { "/projects/#{project.id}/packages/nuget/" } + let(:url) { capybara_url(api(api_path)) } + let(:file) { fixture_file_upload('spec/fixtures/dk.png') } + + subject do + HTTParty.put( + url, + basic_auth: { user: user.username, password: personal_access_token.token }, + body: body + ) + end + + RSpec.shared_examples 'rejecting invalid keys' do |key_name:, message: nil| + context "with invalid key #{key_name}" do + let(:body) { { key_name => file, 'package[test][name]' => 'test' } } + + it { expect { subject }.not_to change { Packages::Package.nuget.count } } + + it { expect(subject.code).to eq(500) } + + it { expect(subject.body).to include(message.presence || "invalid field: \"#{key_name}\"") } + end + end + + RSpec.shared_examples 'by rejecting uploads with an invalid key' do + it_behaves_like 'rejecting invalid keys', key_name: 'package[test' + it_behaves_like 'rejecting invalid keys', key_name: '[]' + it_behaves_like 'rejecting invalid keys', key_name: '[package]test' + it_behaves_like 'rejecting invalid keys', key_name: 'package][test]]' + it_behaves_like 'rejecting invalid keys', key_name: 'package[test[nested]]' + end + + # These keys are rejected directly by rack itself. + # The request will not be received by multipart.rb (can't use the 'handling file uploads' shared example) + it_behaves_like 'rejecting invalid keys', key_name: 'x' * 11000, message: 'Puma caught this error: exceeded available parameter key space (RangeError)' + it_behaves_like 'rejecting invalid keys', key_name: 'package[]test', message: 'Puma caught this error: expected Hash (got Array)' + + it_behaves_like 'handling file uploads', 'by rejecting uploads with an invalid key' + end +end diff --git a/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb b/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb index 875e3820011..a1e9ac6e425 100644 --- a/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_with_handler_for_jwt_params_spec.rb @@ -123,15 +123,46 @@ RSpec.describe Gitlab::Middleware::Multipart do end end - context 'with invalid key in parameters' do + context 'with an invalid upload key' do include_context 'with one temporary file for multipart' - let(:rewritten_fields) { rewritten_fields_hash('file' => uploaded_filepath) } - let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'wrong_key', filename: filename, remote_id: remote_id) } + RSpec.shared_examples 'rejecting the invalid key' do |key_in_header:, key_in_upload_params:, error_message:| + let(:rewritten_fields) { rewritten_fields_hash(key_in_header => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: key_in_upload_params, filename: filename, remote_id: remote_id) } - it 'raises an error' do - expect { subject }.to raise_error(RuntimeError, 'Empty JWT param: file.gitlab-workhorse-upload') + it 'raises an error' do + expect { subject }.to raise_error(RuntimeError, error_message) + end end + + it_behaves_like 'rejecting the invalid key', + key_in_header: 'file', + key_in_upload_params: 'wrong_key', + error_message: 'Empty JWT param: file.gitlab-workhorse-upload' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[user]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[user]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar[image[url]]]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar[image[url]]]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'x' * 11000, + key_in_upload_params: 'user[avatar]', + error_message: "invalid field: \"#{'x' * 11000}\"" end context 'with a modified JWT payload' do diff --git a/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb b/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb index 742a5639ace..8c2af775574 100644 --- a/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb +++ b/spec/lib/gitlab/middleware/multipart_with_handler_spec.rb @@ -139,6 +139,58 @@ RSpec.describe Gitlab::Middleware::Multipart do subject end end + + context 'with invalid key in header' do + include_context 'with one temporary file for multipart' + + RSpec.shared_examples 'rejecting the invalid key' do |key_in_header:, key_in_upload_params:, error_message:| + let(:rewritten_fields) { rewritten_fields_hash(key_in_header => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: key_in_upload_params, filename: filename, remote_id: remote_id) } + + it 'raises an error' do + expect { subject }.to raise_error(RuntimeError, error_message) + end + end + + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[user]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[user]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[]avatar', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[]avatar"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'user[avatar[image[url]]]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "user[avatar[image[url]]]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: '[]', + key_in_upload_params: 'user[avatar]', + error_message: 'invalid field: "[]"' + it_behaves_like 'rejecting the invalid key', + key_in_header: 'x' * 11000, + key_in_upload_params: 'user[avatar]', + error_message: "invalid field: \"#{'x' * 11000}\"" + end + + context 'with key with unbalanced brackets in header' do + include_context 'with one temporary file for multipart' + + let(:invalid_key) { 'user[avatar' } + let(:rewritten_fields) { rewritten_fields_hash( invalid_key => uploaded_filepath) } + let(:params) { upload_parameters_for(filepath: uploaded_filepath, key: 'user[avatar]', filename: filename, remote_id: remote_id) } + + it 'builds no UploadedFile' do + expect(app).not_to receive(:call) + + expect { subject }.to raise_error(RuntimeError, "invalid field: \"#{invalid_key}\"") + end + end end end end diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index 1c56e489a94..66ed80a7d61 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -137,11 +137,16 @@ RSpec.describe Gitlab::Regex do it { is_expected.to match('my/awesome/image-1') } it { is_expected.to match('my/awesome/image.test') } it { is_expected.to match('my/awesome/image--test') } - # docker distribution allows for infinite `-` - # https://github.com/docker/distribution/blob/master/reference/regexp.go#L13 - # but we have a range of 0,10 to add a reasonable limit. - it { is_expected.not_to match('my/image-----------test') } + it { is_expected.to match('my/image__test') } + # this example tests for catastrophic backtracking + it { is_expected.to match('user1/project/a_bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb------------x') } + it { is_expected.not_to match('user1/project/a_bbbbb-------------') } it { is_expected.not_to match('my/image-.test') } + it { is_expected.not_to match('my/image___test') } + it { is_expected.not_to match('my/image_.test') } + it { is_expected.not_to match('my/image_-test') } + it { is_expected.not_to match('my/image..test') } + it { is_expected.not_to match('my/image\ntest') } it { is_expected.not_to match('.my/image') } it { is_expected.not_to match('my/image.') } end diff --git a/spec/lib/gitlab/utils_spec.rb b/spec/lib/gitlab/utils_spec.rb index 1eaceec1d8a..36257a0605b 100644 --- a/spec/lib/gitlab/utils_spec.rb +++ b/spec/lib/gitlab/utils_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::Utils do + using RSpec::Parameterized::TableSyntax + delegate :to_boolean, :boolean_to_yes_no, :slugify, :random_string, :which, :ensure_array_from_string, :to_exclusive_sentence, :bytes_to_megabytes, :append_path, :check_path_traversal!, :allowlisted?, :check_allowed_absolute_path!, :decode_path, :ms_to_round_sec, to: :described_class @@ -50,6 +52,10 @@ RSpec.describe Gitlab::Utils do expect(check_path_traversal!('dir/..foo.rb')).to eq('dir/..foo.rb') expect(check_path_traversal!('dir/.foo.rb')).to eq('dir/.foo.rb') end + + it 'does nothing for a non-string' do + expect(check_path_traversal!(nil)).to be_nil + end end describe '.allowlisted?' do @@ -448,4 +454,29 @@ RSpec.describe Gitlab::Utils do end end end + + describe '.valid_brackets?' do + where(:input, :allow_nested, :valid) do + 'no brackets' | true | true + 'no brackets' | false | true + 'user[avatar]' | true | true + 'user[avatar]' | false | true + 'user[avatar][friends]' | true | true + 'user[avatar][friends]' | false | true + 'user[avatar[image[url]]]' | true | true + 'user[avatar[image[url]]]' | false | false + 'user[avatar[]friends]' | true | true + 'user[avatar[]friends]' | false | false + 'user[avatar]]' | true | false + 'user[avatar]]' | false | false + 'user][avatar]]' | true | false + 'user][avatar]]' | false | false + 'user[avatar' | true | false + 'user[avatar' | false | false + end + + with_them do + it { expect(described_class.valid_brackets?(input, allow_nested: allow_nested)).to eq(valid) } + end + end end diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index f669483b5a4..a532b8e59f2 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -166,6 +166,16 @@ RSpec.describe API::Internal::Kubernetes do ) ) end + + context 'repository is for project members only' do + let(:project) { create(:project, :public, :repository_private) } + + it 'returns 404' do + send_request(params: { id: project.id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end context 'project is private' do @@ -190,7 +200,7 @@ RSpec.describe API::Internal::Kubernetes do context 'project does not exist' do it 'returns 404' do - send_request(params: { id: 0 }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) + send_request(params: { id: non_existing_record_id }, headers: { 'Authorization' => "Bearer #{agent_token.token}" }) expect(response).to have_gitlab_http_status(:not_found) end diff --git a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb index 12bcbb8b812..7126d3ace96 100644 --- a/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb +++ b/spec/support/shared_examples/uploaders/gitlab_uploader_shared_examples.rb @@ -14,6 +14,7 @@ end RSpec.shared_examples "builds correct paths" do |**patterns| let(:patterns) { patterns } + let(:fixture) { File.join('spec', 'fixtures', 'rails_sample.jpg') } before do allow(subject).to receive(:filename).and_return('<filename>') @@ -55,4 +56,15 @@ RSpec.shared_examples "builds correct paths" do |**patterns| let(:target) { subject.class } end end + + describe "path traversal exploits" do + before do + allow(subject).to receive(:filename).and_return("3bc58d54542d6a5efffa9a87554faac0254f73f675b337899ea869f6d38b7371/122../../../../../../../../.ssh/authorized_keys") + end + + it "throws an exception" do + expect { subject.cache!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError) + expect { subject.store!(fixture_file_upload(fixture)) }.to raise_error(Gitlab::Utils::PathTraversalAttackError) + end + end end diff --git a/spec/uploaders/import_export_uploader_spec.rb b/spec/uploaders/import_export_uploader_spec.rb index b1fdcf067c6..cb7a89193e6 100644 --- a/spec/uploaders/import_export_uploader_spec.rb +++ b/spec/uploaders/import_export_uploader_spec.rb @@ -24,9 +24,14 @@ RSpec.describe ImportExportUploader do include_context 'with storage', described_class::Store::REMOTE - it_behaves_like 'builds correct paths', - store_dir: %r[import_export_upload/import_file/], - upload_path: %r[import_export_upload/import_file/] + patterns = { + store_dir: %r[import_export_upload/import_file/], + upload_path: %r[import_export_upload/import_file/] + } + + it_behaves_like 'builds correct paths', patterns do + let(:fixture) { File.join('spec', 'fixtures', 'group_export.tar.gz') } + end describe '#move_to_store' do it 'returns false' do diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore index 259148fa18f..259148fa18f 100755..100644 --- a/vendor/gitignore/C++.gitignore +++ b/vendor/gitignore/C++.gitignore diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore index a1c2a238a96..a1c2a238a96 100755..100644 --- a/vendor/gitignore/Java.gitignore +++ b/vendor/gitignore/Java.gitignore |