From a270d266c5ec39a97a06ee77e78cae979fef4d55 Mon Sep 17 00:00:00 2001
From: Kyle Drake <kyle@kyledrake.net>
Date: Fri, 8 Aug 2025 14:12:45 -0500
Subject: [PATCH] fix for % in filenames

---
 app/api.rb          |  1 -
 models/site.rb      |  3 ++-
 tests/api_tests.rb  | 25 +++++++++++++++++++++++++
 tests/site_tests.rb | 43 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/app/api.rb b/app/api.rb
index 6fa1ac3e..f346cc76 100644
--- a/app/api.rb
+++ b/app/api.rb
@@ -113,7 +113,6 @@ post '/api/upload' do
   end
 
   files.each do |file|
-    file[:filename] = Rack::Utils.unescape file[:filename]
     if !current_site.okay_to_upload?(file)
       api_error 400, 'invalid_file_type', "#{file[:filename]} is not an allowed file type for free sites, supporter required"
     end
diff --git a/models/site.rb b/models/site.rb
index f3635f4e..8d8a0112 100644
--- a/models/site.rb
+++ b/models/site.rb
@@ -1229,7 +1229,8 @@ class Site < Sequel::Model
 
   # https://practicingruby.com/articles/implementing-an-http-file-server?u=dc2ab0f9bb
   def scrubbed_path(path='')
-    path ||= ''
+    path = path.to_s
+
     clean = []
 
     parts = path.to_s.split '/'
diff --git a/tests/api_tests.rb b/tests/api_tests.rb
index bc530459..a4e3174d 100644
--- a/tests/api_tests.rb
+++ b/tests/api_tests.rb
@@ -347,6 +347,31 @@ describe 'api' do
       _(site_file_exists?('te[s]t.jpg')).must_equal true
     end
 
+    it 'succeeds with percent character in filename' do
+      create_site
+      @site.generate_api_key!
+      header 'Authorization', "Bearer #{@site.api_key}"
+
+      test_filenames = [
+        '100% awesome.jpg',
+        'dsfds/50%off.png',
+        '50% sale.txt',
+        'discount%special.png'
+      ]
+
+      test_filenames.each do |filename|
+        post '/api/upload', filename => Rack::Test::UploadedFile.new('./tests/files/test.jpg', 'image/jpeg')
+        _(res[:result]).must_equal 'success'
+        _(site_file_exists?(filename)).must_equal true
+
+        # Verify the filename was stored literally, not URL-decoded
+        @site.reload  # Reload to get fresh site_files
+        site_file = @site.site_files.find { |f| f.path == filename }
+        _(site_file).wont_be_nil
+        _(site_file.path).must_equal filename  # Should be exactly as uploaded
+      end
+    end
+
     it 'succeeds with valid user session' do
       create_site
       post '/api/upload',
diff --git a/tests/site_tests.rb b/tests/site_tests.rb
index 6bf8cb51..0e1a003a 100644
--- a/tests/site_tests.rb
+++ b/tests/site_tests.rb
@@ -143,6 +143,49 @@ describe Site do
     end
   end
 
+  describe 'scrubbed_path' do
+    it 'preserves literal percent characters without URL decoding' do
+      site = Fabricate :site
+
+      test_paths = [
+        '100% awesome.jpg',
+        'derpking/70%off.png',
+        '50% sale.txt',
+        'discount%special.png',
+        'garfield is 100% sexy.jpg',
+        'path/with/100%valid.txt'
+      ]
+
+      test_paths.each do |path|
+        scrubbed = site.scrubbed_path(path)
+        _(scrubbed).must_equal path  # Should be exactly the same - no URL decoding
+      end
+    end
+
+    it 'still handles path traversal and other security issues' do
+      site = Fabricate :site
+
+      # Should still block path traversal
+      _(site.scrubbed_path('../../../etc/passwd')).must_equal 'etc/passwd'
+      _(site.scrubbed_path('../../test')).must_equal 'test'
+
+      # Should still remove empty components and dots
+      _(site.scrubbed_path('/./test/./file.txt')).must_equal 'test/file.txt'
+      _(site.scrubbed_path('test//file.txt')).must_equal 'test/file.txt'
+
+      # But percent characters should be preserved
+      _(site.scrubbed_path('test/70%off.png')).must_equal 'test/70%off.png'
+    end
+
+    it 'raises error for control characters' do
+      site = Fabricate :site
+
+      # Should still raise error for control characters (below ASCII 32)
+      _(proc { site.scrubbed_path("test\x00file.txt") }).must_raise ArgumentError
+      _(proc { site.scrubbed_path("test\x1Ffile.txt") }).must_raise ArgumentError
+    end
+  end
+
   describe 'custom_max_space' do
     it 'should use the custom max space if it is more' do
       site = Fabricate :site