From b7fb465486cb1e46ca1fd423e093ad74bce2148f Mon Sep 17 00:00:00 2001
From: Oscar Di Manno <24323813+oscardimanno@users.noreply.github.com>
Date: Sun, 14 Sep 2025 11:50:31 +0200
Subject: [PATCH] fix: Sanitize filename on download (#9960)

* fix: Sanitize filename on download
* fix: filename encoding in the Content-Disposition header

This improves the handling of the filename* parameter in the Content-Disposition header. Now, the filename* parameter is only used when it differs from the fallback filename

* tests: Add test for the filename* parameter in Content-Disposition
---
 program/lib/Roundcube/rcube_output.php | 8 ++++----
 tests/Framework/OutputTest.php         | 9 +++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/program/lib/Roundcube/rcube_output.php b/program/lib/Roundcube/rcube_output.php
index a74a964ea..e66eb8b9e 100644
--- a/program/lib/Roundcube/rcube_output.php
+++ b/program/lib/Roundcube/rcube_output.php
@@ -256,15 +256,15 @@ abstract class rcube_output
         // @phpstan-ignore-next-line
         if (is_string($filename) && $filename !== '' && strlen($filename) <= 1024) {
             // For non-ascii characters we'll use RFC2231 syntax
-            if (!preg_match('/[^a-zA-Z0-9_.:,?;@+ -]/', $filename)) {
-                $disposition .= "; filename=\"{$filename}\"";
-            } else {
+            $fallback_filename = preg_replace('/[^a-zA-Z0-9_.(),;@+ -]/', '_', $filename);
+            $disposition .= "; filename=\"{$fallback_filename}\"";
+
+            if ($fallback_filename != $filename) {
                 $filename = rawurlencode($filename);
                 $charset = $this->charset;
                 if (!empty($params['charset']) && rcube_charset::is_valid($params['charset'])) {
                     $charset = $params['charset'];
                 }
-
                 $disposition .= "; filename*={$charset}''{$filename}";
             }
         }
diff --git a/tests/Framework/OutputTest.php b/tests/Framework/OutputTest.php
index 5c23d67ef..889c073ea 100644
--- a/tests/Framework/OutputTest.php
+++ b/tests/Framework/OutputTest.php
@@ -26,6 +26,15 @@ class OutputTest extends TestCase
         $this->assertContains('Content-Type: application/octet-stream', $output->headers);
         $this->assertContains('Content-Security-Policy: default-src \'none\'; img-src \'self\'', $output->headers);
 
+        // Test handling of filename*
+        $output->reset();
+        $output->download_headers('test ? test');
+
+        $this->assertCount(3, $output->headers);
+        $this->assertContains('Content-Disposition: attachment; filename="test _ test"; filename*=' . RCUBE_CHARSET . "''" . rawurlencode('test ? test'), $output->headers);
+        $this->assertContains('Content-Type: application/octet-stream', $output->headers);
+        $this->assertContains('Content-Security-Policy: default-src \'none\'; img-src \'self\'', $output->headers);
+
         // Invalid content type
         $output->reset();
         $params = ['type' => 'invalid'];