Skip to content

Commit

Permalink
Fix uploads to AWS S3 folder
Browse files Browse the repository at this point in the history
  • Loading branch information
texpert committed Sep 15, 2024
1 parent 47036f5 commit 5a15640
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 7 deletions.
2 changes: 1 addition & 1 deletion app/helpers/camaleon_cms/uploader_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def upload_file(uploaded_io, settings = {})
res = { error: nil }

# guard against path traversal
return { error: 'Invalid file path' } unless cama_uploader.class.valid_folder_path?(settings[:folder])
return { error: 'Invalid file path' } unless cama_uploader.valid_folder_path?(settings[:folder])

# formats validations
return { error: "#{ct('file_format_error')} (#{settings[:formats]})" } unless cama_uploader.class.validate_file_format(
Expand Down
7 changes: 7 additions & 0 deletions app/uploaders/camaleon_cms_local_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,11 @@ def delete_file(key)
def parse_key(file_path)
file_path.sub(@root_folder, '').cama_fix_media_key
end

def valid_folder_path?(path)
return false unless super
return false if File.absolute_path?(path)

true
end
end
4 changes: 2 additions & 2 deletions app/uploaders/camaleon_cms_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ def self.validate_file_format(key, valid_formats = '*')
valid_formats.include?(File.extname(key).sub('.', '').split('?').first.try(:downcase))
end

def self.valid_folder_path?(path)
def valid_folder_path?(path)
return true if path == '/'

return false if path.include?('..') || File.absolute_path?(path) || path.include?('://')
return false if path.include?('..') || path.include?('://')

true
end
Expand Down
23 changes: 19 additions & 4 deletions spec/helpers/uploader_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,31 @@
end

describe 'file upload with invalid path' do
it 'upload a local file with invalid path of a path traversal try' do
it "doesn't upload a local file with invalid path of a path traversal try" do
expect(upload_file(File.open(@path), { folder: '../../config/initializers' }).keys.include?(:error)).to be(true)
end

it 'upload a local file with invalid URI-like path' do
it "doesn't upload a local file with invalid URI-like path" do
expect(upload_file(File.open(@path), { folder: 'file:///config/initializers' }).keys.include?(:error)).to be(true)
end

it 'upload a local file with an absolute path' do
expect(upload_file(File.open(@path), { folder: '/tmp/config/initializers' }).keys.include?(:error)).to be(true)
context 'with local server' do
before { current_site.set_option('filesystem_type', 'local') }

it "doesn't upload a local file with an absolute path" do
expect(upload_file(File.open(@path), { folder: '/tmp/config/initializers' }).keys.include?(:error)).to be(true)
end
end

context 'with AWS server' do
before do
current_site.set_option('filesystem_type', 's3')
allow_any_instance_of(CamaleonCmsAwsUploader).to receive(:add_file).and_return({})
end

it 'uploads a local file with an absolute path' do
expect(upload_file(File.open(@path), { folder: '/tmp/config/initializers' }).keys.include?(:error)).to be(false)
end
end
end

Expand Down

0 comments on commit 5a15640

Please sign in to comment.