feat(update): enhance backup filename security with random hash, backported from main

Author: thorsten

nginx.conf

@@ -57,6 +57,12 @@ server {
         add_header Cache-Control "public, max-age=31536000, immutable";
     }
 
+    # Block zip files in content directory
+    location ~ ^/content/.*\.zip$ {
+        deny all;
+        return 403;
+    }
+
     # Rewrite logging, should be turned off on production
     rewrite_log on
 

phpmyfaq/.htaccess

@@ -98,6 +98,8 @@ Header set Access-Control-Allow-Headers "Content-Type, Authorization"
     </IfModule>
     # the path to your phpMyFAQ installation
     RewriteBase /
+    # Block zip files in content directory
+    RewriteRule ^content/.*\.zip$ - [F,L]
     # Error pages
     ErrorDocument 404 /index.php?action=404
     # General pages

phpmyfaq/src/phpMyFAQ/Controller/Api/SetupController.php

@@ -31,6 +31,8 @@ class SetupController extends AbstractController
 {
     public function check(Request $request): JsonResponse
     {
+        $this->userIsAuthenticated();
+
         if (empty($request->getContent())) {
             return $this->json(['message' => 'No version given.'], Response::HTTP_BAD_REQUEST);
         }
@@ -67,6 +69,8 @@ public function check(Request $request): JsonResponse
 
     public function backup(Request $request): JsonResponse
     {
+        $this->userIsAuthenticated();
+
         if (empty($request->getContent())) {
             return $this->json(['message' => 'No version given.'], Response::HTTP_BAD_REQUEST);
         }
@@ -92,6 +96,8 @@ public function backup(Request $request): JsonResponse
 
     public function updateDatabase(Request $request): JsonResponse
     {
+        $this->userIsAuthenticated();
+
         if (empty($request->getContent())) {
             return $this->json(['message' => 'No version given.'], Response::HTTP_BAD_REQUEST);
         }

phpmyfaq/src/phpMyFAQ/Setup/Update.php

@@ -27,8 +27,10 @@
 use phpMyFAQ\Setup;
 use phpMyFAQ\System;
 use phpMyFAQ\User;
+use Random\RandomException;
 use RecursiveDirectoryIterator;
 use RecursiveIteratorIterator;
+use SplFileInfo;
 use SplFileObject;
 use Symfony\Component\HttpFoundation\Request;
 use Tivie\HtaccessParser\Exception\SyntaxException;
@@ -49,6 +51,8 @@ class Update extends Setup
     /** @var string[] */
     private array $dryRunQueries = [];
 
+    private ?string $backupFilename = null;
+
     public function __construct(protected System $system, private readonly Configuration $configuration)
     {
         parent::__construct($this->system);
@@ -72,6 +76,7 @@ public function isConfigTableNotAvailable(DatabaseDriver $databaseDriver): bool
     /**
      * Creates a backup of the current config files
      * @throws Exception
+     * @throws RandomException
      */
     public function createConfigBackup(string $configDir): string
     {
@@ -84,19 +89,44 @@ public function createConfigBackup(string $configDir): string
 
         $files = new RecursiveIteratorIterator(
             new RecursiveDirectoryIterator($configDir),
-            RecursiveIteratorIterator::SELF_FIRST
+            RecursiveIteratorIterator::SELF_FIRST,
         );
 
-        foreach ($files as $file) {
-            $file = realpath($file);
-            if (str_contains($file, $configDir . DIRECTORY_SEPARATOR)) {
-                if (is_dir($file)) {
-                    $zipArchive->addEmptyDir(
-                        str_replace($configDir . DIRECTORY_SEPARATOR, '', $file . DIRECTORY_SEPARATOR)
-                    );
-                } elseif (is_file($file)) {
-                    $zipArchive->addFile($file, str_replace($configDir . DIRECTORY_SEPARATOR, '', $file));
-                }
+        foreach ($files as $fileInfo) {
+            if ($fileInfo instanceof SplFileInfo) {
+                $filePath = $fileInfo->getRealPath() ?: $fileInfo->getPathname();
+                $isDir = $fileInfo->isDir();
+                $isFile = $fileInfo->isFile();
+            } else {
+                $filePath = is_string($fileInfo) ? $fileInfo : (string) $fileInfo;
+                $filePath = realpath($filePath) ?: $filePath;
+                $isDir = is_dir($filePath);
+                $isFile = is_file($filePath);
+            }
+
+            if ($filePath === false || $filePath === null || $filePath === '') {
+                continue;
+            }
+
+            // Exclude the zip we are currently writing
+            if ($filePath === $outputZipFile) {
+                continue;
+            }
+
+            // Only include entries inside the config directory
+            if (!str_contains($filePath, $configDir . DIRECTORY_SEPARATOR) && $filePath !== $configDir) {
+                continue;
+            }
+
+            // Compute a relative path inside the archive
+            $relativePath = str_replace($configDir . DIRECTORY_SEPARATOR, '', $filePath);
+            $relativePath = ltrim($relativePath, DIRECTORY_SEPARATOR);
+
+            if ($isDir) {
+                // Ensure directory entries end with a slash
+                $zipArchive->addEmptyDir(rtrim($relativePath, DIRECTORY_SEPARATOR) . DIRECTORY_SEPARATOR);
+            } elseif ($isFile) {
+                $zipArchive->addFile($filePath, $relativePath);
             }
         }
 
@@ -1096,8 +1126,15 @@ private function updateVersion(): void
         $this->configuration->update(['main.currentVersion' => System::getVersion()]);
     }
 
+    /**
+     * @throws RandomException
+     */
     private function getBackupFilename(): string
     {
-        return sprintf('phpmyfaq-config-backup.%s.zip', date('Y-m-d'));
+        if ($this->backupFilename === null) {
+            $randomHash = bin2hex(random_bytes(4)); // 8-character hex string
+            $this->backupFilename = sprintf('phpmyfaq-config-backup.%s.%s.zip', date(format: 'Y-m-d'), $randomHash);
+        }
+        return $this->backupFilename;
     }
 }

tests/phpMyFAQ/Setup/UpdateTest.php

@@ -7,6 +7,7 @@
 use phpMyFAQ\Database\Sqlite3;
 use phpMyFAQ\System;
 use PHPUnit\Framework\TestCase;
+use Random\RandomException;
 
 class UpdateTest extends TestCase
 {
@@ -27,19 +28,40 @@ protected function setUp(): void
 
     /**
      * @throws Exception
+     * @throws RandomException
      */
     public function testCreateConfigBackup(): void
     {
         $this->update->setVersion('4.0.0');
         $configPath = PMF_TEST_DIR . '/content/core/config';
 
+        // Clean up any existing backup files before test
+        $existingFiles = glob($configPath . '/phpmyfaq-config-backup.*.zip');
+        foreach ($existingFiles as $file) {
+            if (file_exists($file)) {
+                unlink($file);
+            }
+        }
+
         $this->update->createConfigBackup($configPath);
 
-        $this->assertFileExists(
-            PMF_TEST_DIR . '/content/core/config/phpmyfaq-config-backup.' . date('Y-m-d') . '.zip'
+        // Find a backup file with a pattern: phpmyfaq-config-backup.YYYY-MM-DD.XXXXXXXX.zip
+        $pattern = PMF_TEST_DIR . '/content/core/config/phpmyfaq-config-backup.' . date(format: 'Y-m-d') . '.*.zip';
+        $files = glob($pattern);
+
+        $this->assertNotEmpty($files, 'Backup file should exist with random hash');
+        $this->assertCount(1, $files, 'Exactly one backup file should exist');
+
+        // Verify filename format: date.hash.zip where hash is 8 hex characters
+        $filename = basename($files[0]);
+        $this->assertMatchesRegularExpression(
+            '/^phpmyfaq-config-backup\.\d{4}-\d{2}-\d{2}\.[0-9a-f]{8}\.zip$/',
+            $filename,
+            'Backup filename should contain 8-character hexadecimal hash'
         );
 
-        unlink(PMF_TEST_DIR . '/content/core/config/phpmyfaq-config-backup.' . date('Y-m-d') . '.zip');
+        // Cleanup
+        unlink($files[0]);
     }
 
     public function testIsConfigTableNotAvailable(): void