From 912e1ff4284eb39fe020b8e823085a2cb7f244fb Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 26 Oct 2016 13:04:31 +0100 Subject: [PATCH] Removes any symlinks before importing a project export file. Also added relevant spec. --- lib/gitlab/import_export/file_importer.rb | 8 ++++ .../import_export/project_tree_restorer.rb | 10 ++++- lib/gitlab/import_export/version_checker.rb | 11 ++++- .../import_export/file_importer_spec.rb | 42 +++++++++++++++++++ .../project_tree_restorer_spec.rb | 14 +++++++ .../import_export/version_checker_spec.rb | 16 ++++++- spec/support/import_export/common_util.rb | 10 +++++ 7 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 spec/lib/gitlab/import_export/file_importer_spec.rb create mode 100644 spec/support/import_export/common_util.rb diff --git a/lib/gitlab/import_export/file_importer.rb b/lib/gitlab/import_export/file_importer.rb index eca6e5b6d512..d296648e801f 100644 --- a/lib/gitlab/import_export/file_importer.rb +++ b/lib/gitlab/import_export/file_importer.rb @@ -43,6 +43,14 @@ def decompress_archive raise Projects::ImportService::Error.new("Unable to decompress #{@archive_file} into #{@shared.export_path}") unless result + remove_symlinks! + end + + def remove_symlinks! + Dir["#{@shared.export_path}/**/*"].each do |path| + FileUtils.rm(path) if File.lstat(path).symlink? + end + true end end diff --git a/lib/gitlab/import_export/project_tree_restorer.rb b/lib/gitlab/import_export/project_tree_restorer.rb index c7b3551b84c5..4f1277a03037 100644 --- a/lib/gitlab/import_export/project_tree_restorer.rb +++ b/lib/gitlab/import_export/project_tree_restorer.rb @@ -9,8 +9,14 @@ def initialize(user:, shared:, project:) end def restore - json = IO.read(@path) - @tree_hash = ActiveSupport::JSON.decode(json) + begin + json = IO.read(@path) + @tree_hash = ActiveSupport::JSON.decode(json) + rescue => e + Rails.logger.error("Import/Export error: #{e.message}") + raise Gitlab::ImportExport::Error.new('Incorrect JSON format') + end + @project_members = @tree_hash.delete('project_members') ActiveRecord::Base.no_touching do diff --git a/lib/gitlab/import_export/version_checker.rb b/lib/gitlab/import_export/version_checker.rb index de3fe6d822e0..bd3c3ee3b2fa 100644 --- a/lib/gitlab/import_export/version_checker.rb +++ b/lib/gitlab/import_export/version_checker.rb @@ -24,12 +24,19 @@ def version_file end def verify_version!(version) - if Gem::Version.new(version) > Gem::Version.new(Gitlab::ImportExport.version) - raise Gitlab::ImportExport::Error.new("Import version mismatch: Required <= #{Gitlab::ImportExport.version} but was #{version}") + if different_version?(version) + raise Gitlab::ImportExport::Error.new("Import version mismatch: Required #{Gitlab::ImportExport.version} but was #{version}") else true end end + + def different_version?(version) + Gem::Version.new(version) != Gem::Version.new(Gitlab::ImportExport.version) + rescue => e + Rails.logger.error("Import/Export error: #{e.message}") + raise Gitlab::ImportExport::Error.new('Incorrect VERSION format') + end end end end diff --git a/spec/lib/gitlab/import_export/file_importer_spec.rb b/spec/lib/gitlab/import_export/file_importer_spec.rb new file mode 100644 index 000000000000..a88ddd17aca6 --- /dev/null +++ b/spec/lib/gitlab/import_export/file_importer_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +describe Gitlab::ImportExport::FileImporter, lib: true do + let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: 'test') } + let(:export_path) { "#{Dir::tmpdir}/file_importer_spec" } + let(:valid_file) { "#{shared.export_path}/valid.json" } + let(:symlink_file) { "#{shared.export_path}/invalid.json" } + let(:subfolder_symlink_file) { "#{shared.export_path}/subfolder/invalid.json" } + + before do + stub_const('Gitlab::ImportExport::FileImporter::MAX_RETRIES', 0) + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path) + allow_any_instance_of(Gitlab::ImportExport::CommandLineUtil).to receive(:untar_zxf).and_return(true) + + setup_files + + described_class.import(archive_file: '', shared: shared) + end + + after do + FileUtils.rm_rf(export_path) + end + + it 'removes symlinks in root folder' do + expect(File.exist?(symlink_file)).to be false + end + + it 'removes symlinks in subfolders' do + expect(File.exist?(subfolder_symlink_file)).to be false + end + + it 'does not remove a valid file' do + expect(File.exist?(valid_file)).to be true + end + + def setup_files + FileUtils.mkdir_p("#{shared.export_path}/subfolder/") + FileUtils.touch(valid_file) + FileUtils.ln_s(valid_file, symlink_file) + FileUtils.ln_s(valid_file, subfolder_symlink_file) + end +end 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 32c0d6462f14..3bb1ee067bd6 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +include ImportExport::CommonUtil describe Gitlab::ImportExport::ProjectTreeRestorer, services: true do describe 'restore project tree' do @@ -72,6 +73,19 @@ expect(Milestone.find_by_description('test milestone').issues).not_to be_empty end + + context 'project.json file access check' do + it 'does not read a symlink' do + Dir.mktmpdir do |tmpdir| + setup_symlink(tmpdir, 'project.json') + allow(shared).to receive(:export_path).and_call_original + + restored_project_json + + expect(shared.errors.first).not_to include('test') + end + end + end end end end diff --git a/spec/lib/gitlab/import_export/version_checker_spec.rb b/spec/lib/gitlab/import_export/version_checker_spec.rb index 90c6d1c67f6e..a3b30dc835bf 100644 --- a/spec/lib/gitlab/import_export/version_checker_spec.rb +++ b/spec/lib/gitlab/import_export/version_checker_spec.rb @@ -1,8 +1,10 @@ require 'spec_helper' +include ImportExport::CommonUtil describe Gitlab::ImportExport::VersionChecker, services: true do + let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: '') } + describe 'bundle a project Git repo' do - let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: '') } let(:version) { Gitlab::ImportExport.version } before do @@ -27,4 +29,16 @@ end end end + + describe 'version file access check' do + it 'does not read a symlink' do + Dir.mktmpdir do |tmpdir| + setup_symlink(tmpdir, 'VERSION') + + described_class.check!(shared: shared) + + expect(shared.errors.first).not_to include('test') + end + end + end end diff --git a/spec/support/import_export/common_util.rb b/spec/support/import_export/common_util.rb new file mode 100644 index 000000000000..2542a59bb00d --- /dev/null +++ b/spec/support/import_export/common_util.rb @@ -0,0 +1,10 @@ +module ImportExport + module CommonUtil + def setup_symlink(tmpdir, symlink_name) + allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(tmpdir) + + File.open("#{tmpdir}/test", 'w') { |file| file.write("test") } + FileUtils.ln_s("#{tmpdir}/test", "#{tmpdir}/#{symlink_name}") + end + end +end