From a1cb3c6a118f04bc9c3aacb215bc6e877d19b92a Mon Sep 17 00:00:00 2001 From: Kyle Drake Date: Mon, 15 Jan 2024 08:59:57 -0600 Subject: [PATCH] add defined file and path length limits --- app/site_files.rb | 10 +++++++++- models/site_file.rb | 12 ++++++++++++ tests/site_file_tests.rb | 22 ++++++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) diff --git a/app/site_files.rb b/app/site_files.rb index ef0b8bb2..fa429c50 100644 --- a/app/site_files.rb +++ b/app/site_files.rb @@ -130,7 +130,9 @@ post '/site_files/upload' do end end - file[:filename] = "#{dir_name.force_encoding('UTF-8')}/#{site.scrubbed_path file[:filename].force_encoding('UTF-8')}" + file_base_name = site.scrubbed_path file[:filename].force_encoding('UTF-8') + + file[:filename] = "#{dir_name.force_encoding('UTF-8')}/#{file_base_name}" if current_site.file_size_too_large? file[:tempfile].size file_upload_response "#{Rack::Utils.escape_html file[:filename]} is too large, upload cancelled." @@ -138,6 +140,12 @@ post '/site_files/upload' do if !site.okay_to_upload? file file_upload_response %{#{Rack::Utils.escape_html file[:filename]}: file type (or content in file) is only supported by supporter accounts. Why We Do This} end + if SiteFile.path_too_long? file[:filename] + file_upload_response "#{Rack::Utils.escape_html file[:filename]}: path is too long, upload cancelled." + end + if SiteFile.name_too_long? file_base_name + file_upload_response "#{Rack::Utils.escape_html file[:filename]}: file name is too long, upload cancelled." + end end uploaded_size = params[:files].collect {|f| f[:tempfile].size}.inject{|sum,x| sum + x } diff --git a/models/site_file.rb b/models/site_file.rb index 40876358..52305766 100644 --- a/models/site_file.rb +++ b/models/site_file.rb @@ -5,10 +5,22 @@ require 'sanitize' class SiteFile < Sequel::Model CLASSIFIER_LIMIT = 1_000_000 CLASSIFIER_WORD_LIMIT = 25 + FILE_PATH_CHARACTER_LIMIT = 300 + FILE_NAME_CHARACTER_LIMIT = 100 unrestrict_primary_key plugin :update_primary_key many_to_one :site + def self.path_too_long?(filename) + return true if filename.length > FILE_PATH_CHARACTER_LIMIT + false + end + + def self.name_too_long?(filename) + return true if filename.length > FILE_NAME_CHARACTER_LIMIT + false + end + def before_destroy if is_directory site.site_files_dataset.where(path: /^#{Regexp.quote path}\//, is_directory: true).all.each do |site_file| diff --git a/tests/site_file_tests.rb b/tests/site_file_tests.rb index 1c9b7547..24643d17 100644 --- a/tests/site_file_tests.rb +++ b/tests/site_file_tests.rb @@ -270,6 +270,28 @@ describe 'site_files' do _(File.exists?(@site.files_path('cache.manifest'))).must_equal true end + it 'fails with filename greater than limit' do + file_path = './tests/files' + (0...SiteFile::FILE_NAME_CHARACTER_LIMIT+1).map { ('a'..'z').to_a[rand(26)] }.join + '.html' + begin + File.open(file_path, "w") do |file| + file.write("derp") + end + + upload 'files[]' => Rack::Test::UploadedFile.new(file_path, 'text/html') + _(last_response.body).must_match /name is too long/i + ensure + FileUtils.rm file_path + end + end + + it 'fails with path greater than limit' do + upload( + 'dir' => (("a" * 50 + "/") * (SiteFile::FILE_PATH_CHARACTER_LIMIT / 50 - 1) + "a" * 50), + 'files[]' => Rack::Test::UploadedFile.new('./tests/files/test.jpg', 'image/jpeg') + ) + _(last_response.body).must_match /path is too long/i + end + it 'works with otf fonts' do upload 'files[]' => Rack::Test::UploadedFile.new('./tests/files/chunkfive.otf', 'application/vnd.ms-opentype') _(File.exists?(@site.files_path('chunkfive.otf'))).must_equal true