diff --git a/app/controllers/admin/product_import_controller.rb b/app/controllers/admin/product_import_controller.rb index c71e3ad015..657af94684 100644 --- a/app/controllers/admin/product_import_controller.rb +++ b/app/controllers/admin/product_import_controller.rb @@ -4,6 +4,8 @@ require 'roo' module Admin class ProductImportController < Spree::Admin::BaseController + TMPDIR_PREFIX = "product_import-" + before_action :validate_upload_presence, except: %i[index guide validate_data] def index @@ -101,8 +103,7 @@ module Admin def save_uploaded_file(upload) extension = File.extname(upload.original_filename) - directory = Dir.mktmpdir 'product_import' - File.open(File.join(directory, "import#{extension}"), 'wb') do |f| + File.open(File.join(mktmpdir, "import#{extension}"), 'wb') do |f| data = UploadSanitizer.new(upload.read).call f.write(data) f.path @@ -126,6 +127,16 @@ module Admin ProductImport::ProductImporter end + def mktmpdir + tmpdir = tmpdir_base + SecureRandom.hex(6) + Dir.mkdir(tmpdir) + tmpdir + end + + def tmpdir_base + Rails.root.join('tmp', TMPDIR_PREFIX).to_s + end + def file_path @file_path ||= validate_file_path(sanitize_file_path(params[:filepath])) end @@ -134,8 +145,9 @@ module Admin FilePathSanitizer.new.sanitize(file_path, on_error: method(:raise_invalid_file_path)) end + # Ensure file is under the safe tmp directory def validate_file_path(file_path) - return file_path if file_path.to_s.match?(TEMP_FILE_PATH_REGEX) + return file_path if file_path.to_s.match?(%r{^#{tmpdir_base}[A-Za-z0-9-]*/import\.csv$}) raise_invalid_file_path end @@ -145,6 +157,5 @@ module Admin notice: I18n.t(:product_import_no_data_in_spreadsheet_notice) raise 'Invalid File Path' end - TEMP_FILE_PATH_REGEX = %r{^/tmp/product_import[A-Za-z0-9-]*/import\.csv$} end end diff --git a/spec/controllers/admin/product_import_controller_spec.rb b/spec/controllers/admin/product_import_controller_spec.rb index 060c951939..ac6cf560f1 100644 --- a/spec/controllers/admin/product_import_controller_spec.rb +++ b/spec/controllers/admin/product_import_controller_spec.rb @@ -4,6 +4,8 @@ require 'spec_helper' RSpec.describe Admin::ProductImportController, type: :controller do describe 'validate_file_path' do + let(:tmp_directory_base) { Rails.root.join("tmp/product_import-") } + before do # Avoid error on redirect_to allow(controller).to receive(:raise_invalid_file_path).and_return(false) @@ -11,39 +13,39 @@ RSpec.describe Admin::ProductImportController, type: :controller do context 'file extension' do it 'should authorize csv extension' do - path = '/tmp/product_import123/import.csv' + path = "#{tmp_directory_base}123/import.csv" expect(controller.__send__(:validate_file_path, path)).to be_truthy end it 'should reject other extensions' do - path = '/tmp/product_import123/import.pdf' + path = "#{tmp_directory_base}123/import.pdf" expect(controller.__send__(:validate_file_path, path)).to be_falsey - path1 = '/tmp/product_import123/import.xslx' + path1 = "#{tmp_directory_base}123/import.xslx" expect(controller.__send__(:validate_file_path, path1)).to be_falsey end end context 'folder path' do it 'should authorize valid paths' do - path = '/tmp/product_import123/import.csv' + path = "#{tmp_directory_base}123/import.csv" expect(controller.__send__(:validate_file_path, path)).to be_truthy - path1 = '/tmp/product_importabc/import.csv' + path1 = "#{tmp_directory_base}abc/import.csv" expect(controller.__send__(:validate_file_path, path1)).to be_truthy - path2 = '/tmp/product_importABC-abc-123/import.csv' + path2 = "#{tmp_directory_base}ABC-abc-123/import.csv" expect(controller.__send__(:validate_file_path, path2)).to be_truthy end it 'should reject invalid paths' do - path = '/tmp/product_import123/../etc/import.csv' + path = "#{tmp_directory_base}123/../etc/import.csv" expect(controller.__send__(:validate_file_path, path)).to be_falsey - path1 = '/tmp/product_import../etc/import.csv' + path1 = "#{tmp_directory_base}../etc/import.csv" expect(controller.__send__(:validate_file_path, path1)).to be_falsey - path2 = '/tmp/product_import132%2F..%2Fetc%2F/import.csv' + path2 = "#{tmp_directory_base}132%2F..%2Fetc%2F/import.csv" expect(controller.__send__(:validate_file_path, path2)).to be_falsey - path3 = '/etc/tmp/product_import123/import.csv' + path3 = "/etc#{tmp_directory_base}123/import.csv" expect(controller.__send__(:validate_file_path, path3)).to be_falsey end end