Mitigate arbitrary path traversal in download_private_file

CHANGELOG.md

@@ -7,6 +7,8 @@
 - Fix colorpicker missing admin asset, adding it to `admin-manifest.css`
 - **Security fix:** Mitigate arbitrary path write in uploader (GHSL-2024-182)
   - Thanks [Peter Stöckli](https://github.com/p-) for reporting and providing clear reproduction steps
+- **Security fix:** Mitigate arbitrary path traversal in download_private_file (GHSL-2024-183)
+  - Thanks [Peter Stöckli](https://github.com/p-) for reporting and providing clear reproduction steps
 
 ## [2.8.0](https://github.com/owen2345/camaleon-cms/tree/2.8.0) (2024-07-26)
 - Use jQuery 2.x - 2.2.4

app/controllers/camaleon_cms/admin/media_controller.rb

@@ -30,6 +30,10 @@ def download_private_file
 
         file = cama_uploader.fetch_file("private/#{params[:file]}")
 
+        if file.is_a?(Hash) && file[:error].present?
+          return render plain: helpers.sanitize(file[:error])
+        end
+
         send_file file, disposition: 'inline'
       end
 

app/uploaders/camaleon_cms_local_uploader.rb

@@ -25,6 +25,8 @@ def browser_files(prefix = '/', _objects = {})
   end
 
   def fetch_file(file_name)
+    return { error: 'Invalid file path' } if file_name.include?('..')
+
     raise ActionController::RoutingError, 'File not found' unless file_exists?(file_name)
 
     file_name

spec/requests/download_private_file_spec.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe 'Media requests', type: :request do
+  init_site
+
+  describe 'Download private file' do
+    let(:current_site) { Cama::Site.first.decorate }
+
+    before do
+      allow_any_instance_of(CamaleonCms::Admin::MediaController).to receive(:cama_authenticate)
+      allow_any_instance_of(CamaleonCms::Admin::MediaController).to receive(:current_site).and_return(current_site)
+    end
+
+    context 'when the file path is valid and file exists' do
+      before do
+        expect_any_instance_of(CamaleonCmsLocalUploader).to receive(:fetch_file).and_return('some_file')
+
+        allow_any_instance_of(CamaleonCms::Admin::MediaController).to receive(:send_file)
+        allow_any_instance_of(CamaleonCms::Admin::MediaController).to receive(:default_render)
+      end
+
+      it 'allows the file to be downloaded' do
+        expect_any_instance_of(CamaleonCms::Admin::MediaController).to receive(:send_file).with('some_file', disposition: 'inline')
+
+        get '/admin/media/download_private_file', params: { file: 'some_file' }
+      end
+    end
+
+    context 'when file path is invalid' do
+      it 'returns an error' do
+        get '/admin/media/download_private_file', params: { file: './../../../../../etc/passwd' }
+
+        expect(response.body).to include('Invalid file path')
+      end
+    end
+
+    context 'when the file is not found' do
+      it 'returns an error' do
+        expect { get '/admin/media/download_private_file', params: { file: 'passwd' } }
+          .to raise_error(ActionController::RoutingError, 'File not found')
+      end
+    end
+  end
+end