From 99043600ee881fd8581185e7590604d9882382cd Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 25 Mar 2026 16:52:06 +0100 Subject: [PATCH] fix(backup): validate MongoDB collection names in backup input Add validateDatabasesBackupInput() helper that properly parses all database backup formats including MongoDB's "db:col1,col2|db2:col3" and validates each component individually. - Validate and escape collection names in DatabaseBackupJob - Replace comma-only split in BackupEdit with format-aware validation - Add input validation in API create_backup and update_backup endpoints - Add unit tests for collection name and multi-format validation Co-Authored-By: Claude Opus 4.6 --- .../Controllers/Api/DatabasesController.php | 24 ++++++++ app/Jobs/DatabaseBackupJob.php | 12 +++- app/Livewire/Project/Database/BackupEdit.php | 16 +---- bootstrap/helpers/shared.php | 53 ++++++++++++++++ tests/Unit/DatabaseBackupSecurityTest.php | 61 +++++++++++++++++++ 5 files changed, 150 insertions(+), 16 deletions(-) diff --git a/app/Http/Controllers/Api/DatabasesController.php b/app/Http/Controllers/Api/DatabasesController.php index 700055fcc..44b66e57e 100644 --- a/app/Http/Controllers/Api/DatabasesController.php +++ b/app/Http/Controllers/Api/DatabasesController.php @@ -792,6 +792,18 @@ public function create_backup(Request $request) } } + // Validate databases_to_backup input + if (! empty($backupData['databases_to_backup'])) { + try { + validateDatabasesBackupInput($backupData['databases_to_backup']); + } catch (\Exception $e) { + return response()->json([ + 'message' => 'Validation failed.', + 'errors' => ['databases_to_backup' => [$e->getMessage()]], + ], 422); + } + } + // Add required fields $backupData['database_id'] = $database->id; $backupData['database_type'] = $database->getMorphClass(); @@ -997,6 +1009,18 @@ public function update_backup(Request $request) unset($backupData['s3_storage_uuid']); } + // Validate databases_to_backup input + if (! empty($backupData['databases_to_backup'])) { + try { + validateDatabasesBackupInput($backupData['databases_to_backup']); + } catch (\Exception $e) { + return response()->json([ + 'message' => 'Validation failed.', + 'errors' => ['databases_to_backup' => [$e->getMessage()]], + ], 422); + } + } + $backupConfig->update($backupData); if ($request->backup_now) { diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index 041d31bad..d86986fad 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -524,10 +524,18 @@ private function backup_standalone_mongodb(string $databaseWithCollections): voi $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --db $escapedDatabaseName --gzip --archive > $this->backup_location"; } } else { + // Validate and escape each collection name + $escapedCollections = $collectionsToExclude->map(function ($collection) { + $collection = trim($collection); + validateShellSafePath($collection, 'collection name'); + + return escapeshellarg($collection); + }); + if (str($this->database->image)->startsWith('mongo:4')) { - $commands[] = "docker exec $this->container_name mongodump --uri=$url --gzip --excludeCollection ".$collectionsToExclude->implode(' --excludeCollection ')." --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --uri=$url --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location"; } else { - $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --db $escapedDatabaseName --gzip --excludeCollection ".$collectionsToExclude->implode(' --excludeCollection ')." --archive > $this->backup_location"; + $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --db $escapedDatabaseName --gzip --excludeCollection ".$escapedCollections->implode(' --excludeCollection ')." --archive > $this->backup_location"; } } } diff --git a/app/Livewire/Project/Database/BackupEdit.php b/app/Livewire/Project/Database/BackupEdit.php index c24e2a3f1..0fff2bd03 100644 --- a/app/Livewire/Project/Database/BackupEdit.php +++ b/app/Livewire/Project/Database/BackupEdit.php @@ -105,21 +105,9 @@ public function syncData(bool $toModel = false) $this->backup->s3_storage_id = $this->s3StorageId; // Validate databases_to_backup to prevent command injection + // Handles all formats including MongoDB's "db:col1,col2|db2:col3" if (filled($this->databasesToBackup)) { - $databases = str($this->databasesToBackup)->explode(','); - foreach ($databases as $index => $db) { - $dbName = trim($db); - try { - validateShellSafePath($dbName, 'database name'); - } catch (\Exception $e) { - // Provide specific error message indicating which database failed validation - $position = $index + 1; - throw new \Exception( - "Database #{$position} ('{$dbName}') validation failed: ". - $e->getMessage() - ); - } - } + validateDatabasesBackupInput($this->databasesToBackup); } $this->backup->databases_to_backup = $this->databasesToBackup; diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index a8cffcaff..84472a07e 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -148,6 +148,59 @@ function validateShellSafePath(string $input, string $context = 'path'): string return $input; } +/** + * Validate that a databases_to_backup input string is safe from command injection. + * + * Supports all database formats: + * - PostgreSQL/MySQL/MariaDB: "db1,db2,db3" + * - MongoDB: "db1:col1,col2|db2:col3,col4" + * + * Validates each database name AND collection name individually against shell metacharacters. + * + * @param string $input The databases_to_backup string + * @return string The validated input + * + * @throws \Exception If any component contains dangerous characters + */ +function validateDatabasesBackupInput(string $input): string +{ + // Split by pipe (MongoDB multi-db separator) + $databaseEntries = explode('|', $input); + + foreach ($databaseEntries as $entry) { + $entry = trim($entry); + if ($entry === '' || $entry === 'all' || $entry === '*') { + continue; + } + + if (str_contains($entry, ':')) { + // MongoDB format: dbname:collection1,collection2 + $databaseName = str($entry)->before(':')->value(); + $collections = str($entry)->after(':')->explode(','); + + validateShellSafePath($databaseName, 'database name'); + + foreach ($collections as $collection) { + $collection = trim($collection); + if ($collection !== '') { + validateShellSafePath($collection, 'collection name'); + } + } + } else { + // Simple format: just a database name (may contain commas for non-Mongo) + $databases = explode(',', $entry); + foreach ($databases as $db) { + $db = trim($db); + if ($db !== '' && $db !== 'all' && $db !== '*') { + validateShellSafePath($db, 'database name'); + } + } + } + } + + return $input; +} + /** * Validate that a string is a safe git ref (commit SHA, branch name, tag, or HEAD). * diff --git a/tests/Unit/DatabaseBackupSecurityTest.php b/tests/Unit/DatabaseBackupSecurityTest.php index 6fb0bb4b9..90940c174 100644 --- a/tests/Unit/DatabaseBackupSecurityTest.php +++ b/tests/Unit/DatabaseBackupSecurityTest.php @@ -81,3 +81,64 @@ expect(fn () => validateShellSafePath('test123', 'database name')) ->not->toThrow(Exception::class); }); + +// --- MongoDB collection name validation tests --- + +test('mongodb collection name rejects command substitution injection', function () { + expect(fn () => validateShellSafePath('$(touch /tmp/pwned)', 'collection name')) + ->toThrow(Exception::class); +}); + +test('mongodb collection name rejects backtick injection', function () { + expect(fn () => validateShellSafePath('`id > /tmp/pwned`', 'collection name')) + ->toThrow(Exception::class); +}); + +test('mongodb collection name rejects semicolon injection', function () { + expect(fn () => validateShellSafePath('col1; rm -rf /', 'collection name')) + ->toThrow(Exception::class); +}); + +test('mongodb collection name rejects ampersand injection', function () { + expect(fn () => validateShellSafePath('col1 & whoami', 'collection name')) + ->toThrow(Exception::class); +}); + +test('mongodb collection name rejects redirect injection', function () { + expect(fn () => validateShellSafePath('col1 > /tmp/pwned', 'collection name')) + ->toThrow(Exception::class); +}); + +test('validateDatabasesBackupInput validates mongodb format with collection names', function () { + // Valid MongoDB formats should pass + expect(fn () => validateDatabasesBackupInput('mydb')) + ->not->toThrow(Exception::class); + + expect(fn () => validateDatabasesBackupInput('mydb:col1,col2')) + ->not->toThrow(Exception::class); + + expect(fn () => validateDatabasesBackupInput('db1:col1,col2|db2:col3')) + ->not->toThrow(Exception::class); + + expect(fn () => validateDatabasesBackupInput('all')) + ->not->toThrow(Exception::class); +}); + +test('validateDatabasesBackupInput rejects injection in collection names', function () { + // Command substitution in collection name + expect(fn () => validateDatabasesBackupInput('mydb:$(touch /tmp/pwned)')) + ->toThrow(Exception::class); + + // Backtick injection in collection name + expect(fn () => validateDatabasesBackupInput('mydb:`id`')) + ->toThrow(Exception::class); + + // Semicolon in collection name + expect(fn () => validateDatabasesBackupInput('mydb:col1;rm -rf /')) + ->toThrow(Exception::class); +}); + +test('validateDatabasesBackupInput rejects injection in database name within mongo format', function () { + expect(fn () => validateDatabasesBackupInput('$(whoami):col1,col2')) + ->toThrow(Exception::class); +});