mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-02-27 01:43:22 +00:00
add file path validation
This commit is contained in:
committed by
Maikel Linke
parent
a0d05b26d1
commit
4d5ba6a7e6
@@ -658,6 +658,7 @@ Metrics/ClassLength:
|
||||
- 'app/components/products_table_component.rb'
|
||||
- 'app/controllers/admin/enterprises_controller.rb'
|
||||
- 'app/controllers/admin/order_cycles_controller.rb'
|
||||
- 'app/controllers/admin/product_import_controller.rb'
|
||||
- 'app/controllers/admin/resource_controller.rb'
|
||||
- 'app/controllers/admin/schedules_controller.rb'
|
||||
- 'app/controllers/admin/subscriptions_controller.rb'
|
||||
|
||||
@@ -37,7 +37,7 @@ module Admin
|
||||
end
|
||||
|
||||
def reset_absent_products
|
||||
@importer = ProductImport::ProductImporter.new(File.new(params[:filepath]),
|
||||
@importer = ProductImport::ProductImporter.new(File.new(file_path),
|
||||
spree_current_user, import_into: params[:import_into], enterprises_to_reset: params[:enterprises_to_reset], updated_ids: params[:updated_ids], settings: params[:settings])
|
||||
|
||||
if params.key?(:enterprises_to_reset) && params.key?(:updated_ids)
|
||||
@@ -56,7 +56,7 @@ module Admin
|
||||
end
|
||||
|
||||
def process_data(method)
|
||||
@importer = ProductImport::ProductImporter.new(File.new(params[:filepath]),
|
||||
@importer = ProductImport::ProductImporter.new(File.new(file_path),
|
||||
spree_current_user, start: params[:start], end: params[:end], settings: params[:settings])
|
||||
|
||||
begin
|
||||
@@ -112,5 +112,27 @@ module Admin
|
||||
def model_class
|
||||
ProductImport::ProductImporter
|
||||
end
|
||||
|
||||
def file_path
|
||||
@file_path ||= validate_file_path(sanitize_file_path(params[:filepath]))
|
||||
end
|
||||
|
||||
def sanitize_file_path(file_path)
|
||||
pathname = Pathname.new(file_path)
|
||||
raise_invalid_file_path unless pathname.file?
|
||||
pathname.realpath
|
||||
end
|
||||
|
||||
def validate_file_path(file_path)
|
||||
return file_path if file_path.to_s.match?(TEMP_FILE_PATH_REGEX)
|
||||
|
||||
raise_invalid_file_path
|
||||
end
|
||||
|
||||
def raise_invalid_file_path
|
||||
redirect_to '/admin/product_import', 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
|
||||
|
||||
82
spec/controllers/admin/product_import_controller_spec.rb
Normal file
82
spec/controllers/admin/product_import_controller_spec.rb
Normal file
@@ -0,0 +1,82 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require 'spec_helper'
|
||||
|
||||
describe Admin::ProductImportController, type: :controller do
|
||||
describe 'validate_file_path' do
|
||||
context 'file extension' do
|
||||
it 'should authorize csv extension' do
|
||||
path = '/tmp/product_import123/import.csv'
|
||||
expect(controller.__send__(:validate_file_path, path)).to be_truthy
|
||||
end
|
||||
|
||||
it 'should reject other extensions' do
|
||||
allow(controller).to receive(:raise_invalid_file_path).and_return(false)
|
||||
path = '/tmp/product_import123/import.pdf'
|
||||
expect(controller.__send__(:validate_file_path, path)).to be_falsey
|
||||
path1 = '/tmp/product_import123/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'
|
||||
expect(controller.__send__(:validate_file_path, path)).to be_truthy
|
||||
path1 = '/tmp/product_importabc/import.csv'
|
||||
expect(controller.__send__(:validate_file_path, path1)).to be_truthy
|
||||
path2 = '/tmp/product_importABC-abc-123/import.csv'
|
||||
expect(controller.__send__(:validate_file_path, path2)).to be_truthy
|
||||
end
|
||||
|
||||
it 'should reject invalid paths' do
|
||||
allow(controller).to receive(:raise_invalid_file_path).and_return(false)
|
||||
path = '/tmp/product_import123/../etc/import.csv'
|
||||
expect(controller.__send__(:validate_file_path, path)).to be_falsey
|
||||
|
||||
path1 = '/tmp/product_import../etc/import.csv'
|
||||
expect(controller.__send__(:validate_file_path, path1)).to be_falsey
|
||||
|
||||
path2 = '/tmp/product_import132%2F..%2Fetc%2F/import.csv'
|
||||
expect(controller.__send__(:validate_file_path, path2)).to be_falsey
|
||||
|
||||
path3 = '/etc/tmp/product_import123/import.csv'
|
||||
expect(controller.__send__(:validate_file_path, path3)).to be_falsey
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'sanitize_file_path' do
|
||||
let(:folder_path){ '/tmp/product_import123' }
|
||||
let(:file_path) { "#{folder_path}/import.csv" }
|
||||
|
||||
before do
|
||||
FileUtils.mkdir_p(folder_path)
|
||||
File.new(file_path, 'w') unless File.exist?(file_path)
|
||||
end
|
||||
|
||||
def subject(path)
|
||||
# expose the private method sanitize_file_path
|
||||
described_class.class_eval { public :sanitize_file_path }
|
||||
described_class.new.__send__(:sanitize_file_path, path)
|
||||
end
|
||||
|
||||
it 'should covert relative path to absolute' do
|
||||
path = "/tmp/product_import123/import.csv"
|
||||
expect(subject(path).to_s).to eq file_path
|
||||
|
||||
path1 = "/../../tmp/product_import123/import.csv"
|
||||
expect(subject(path1).to_s).to eq file_path
|
||||
|
||||
path2 = "/etc/../../../tmp/product_import123/import.csv"
|
||||
expect(subject(path2).to_s).to eq file_path
|
||||
end
|
||||
|
||||
it "raise an exception if the file doesn't exist" do
|
||||
path = '/tmp/product_import123/import1.csv'
|
||||
allow_any_instance_of(described_class).to receive(:raise_invalid_file_path)
|
||||
.and_raise('Invalid File Path')
|
||||
expect{ subject(path) }.to raise_error('Invalid File Path')
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user