feat: add validation methods for S3 bucket names, paths, and server paths; update import logic to prevent command injection

This commit is contained in:
Andras Bacsai 2025-11-25 16:40:35 +01:00
parent 6c030d96f2
commit 9113ed714f
9 changed files with 272 additions and 55 deletions

View File

@ -22,11 +22,11 @@ class RestoreJobFinished
$commands = [];
if (isSafeTmpPath($scriptPath)) {
$commands[] = "docker exec {$container} sh -c 'rm ".escapeshellarg($scriptPath)." 2>/dev/null || true'";
$commands[] = 'docker exec '.escapeshellarg($container)." sh -c 'rm ".escapeshellarg($scriptPath)." 2>/dev/null || true'";
}
if (isSafeTmpPath($tmpPath)) {
$commands[] = "docker exec {$container} sh -c 'rm ".escapeshellarg($tmpPath)." 2>/dev/null || true'";
$commands[] = 'docker exec '.escapeshellarg($container)." sh -c 'rm ".escapeshellarg($tmpPath)." 2>/dev/null || true'";
}
if (! empty($commands)) {

View File

@ -13,6 +13,92 @@ class Import extends Component
{
use AuthorizesRequests;
/**
* Validate that a string is safe for use as an S3 bucket name.
* Allows alphanumerics, dots, dashes, and underscores.
*/
private function validateBucketName(string $bucket): bool
{
return preg_match('/^[a-zA-Z0-9.\-_]+$/', $bucket) === 1;
}
/**
* Validate that a string is safe for use as an S3 path.
* Allows alphanumerics, dots, dashes, underscores, slashes, and common file characters.
*/
private function validateS3Path(string $path): bool
{
// Must not be empty
if (empty($path)) {
return false;
}
// Must not contain dangerous shell metacharacters or command injection patterns
$dangerousPatterns = [
'..', // Directory traversal
'$(', // Command substitution
'`', // Backtick command substitution
'|', // Pipe
';', // Command separator
'&', // Background/AND
'>', // Redirect
'<', // Redirect
"\n", // Newline
"\r", // Carriage return
"\0", // Null byte
"'", // Single quote
'"', // Double quote
'\\', // Backslash
];
foreach ($dangerousPatterns as $pattern) {
if (str_contains($path, $pattern)) {
return false;
}
}
// Allow alphanumerics, dots, dashes, underscores, slashes, spaces, plus, equals, at
return preg_match('/^[a-zA-Z0-9.\-_\/\s+@=]+$/', $path) === 1;
}
/**
* Validate that a string is safe for use as a file path on the server.
*/
private function validateServerPath(string $path): bool
{
// Must be an absolute path
if (! str_starts_with($path, '/')) {
return false;
}
// Must not contain dangerous shell metacharacters or command injection patterns
$dangerousPatterns = [
'..', // Directory traversal
'$(', // Command substitution
'`', // Backtick command substitution
'|', // Pipe
';', // Command separator
'&', // Background/AND
'>', // Redirect
'<', // Redirect
"\n", // Newline
"\r", // Carriage return
"\0", // Null byte
"'", // Single quote
'"', // Double quote
'\\', // Backslash
];
foreach ($dangerousPatterns as $pattern) {
if (str_contains($path, $pattern)) {
return false;
}
}
// Allow alphanumerics, dots, dashes, underscores, slashes, and spaces
return preg_match('/^[a-zA-Z0-9.\-_\/\s]+$/', $path) === 1;
}
public bool $unsupported = false;
public $resource;
@ -160,8 +246,16 @@ EOD;
public function checkFile()
{
if (filled($this->customLocation)) {
// Validate the custom location to prevent command injection
if (! $this->validateServerPath($this->customLocation)) {
$this->dispatch('error', 'Invalid file path. Path must be absolute and contain only safe characters (alphanumerics, dots, dashes, underscores, slashes).');
return;
}
try {
$result = instant_remote_process(["ls -l {$this->customLocation}"], $this->server, throwError: false);
$escapedPath = escapeshellarg($this->customLocation);
$result = instant_remote_process(["ls -l {$escapedPath}"], $this->server, throwError: false);
if (blank($result)) {
$this->dispatch('error', 'The file does not exist or has been deleted.');
@ -197,8 +291,15 @@ EOD;
Storage::delete($backupFileName);
$this->importCommands[] = "docker cp {$tmpPath} {$this->container}:{$tmpPath}";
} elseif (filled($this->customLocation)) {
// Validate the custom location to prevent command injection
if (! $this->validateServerPath($this->customLocation)) {
$this->dispatch('error', 'Invalid file path. Path must be absolute and contain only safe characters.');
return;
}
$tmpPath = '/tmp/restore_'.$this->resource->uuid;
$this->importCommands[] = "docker cp {$this->customLocation} {$this->container}:{$tmpPath}";
$escapedCustomLocation = escapeshellarg($this->customLocation);
$this->importCommands[] = "docker cp {$escapedCustomLocation} {$this->container}:{$tmpPath}";
} else {
$this->dispatch('error', 'The file does not exist or has been deleted.');
@ -208,38 +309,7 @@ EOD;
// Copy the restore command to a script file
$scriptPath = "/tmp/restore_{$this->resource->uuid}.sh";
switch ($this->resource->getMorphClass()) {
case \App\Models\StandaloneMariadb::class:
$restoreCommand = $this->mariadbRestoreCommand;
if ($this->dumpAll) {
$restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | mariadb -u root -p\$MARIADB_ROOT_PASSWORD";
} else {
$restoreCommand .= " < {$tmpPath}";
}
break;
case \App\Models\StandaloneMysql::class:
$restoreCommand = $this->mysqlRestoreCommand;
if ($this->dumpAll) {
$restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | mysql -u root -p\$MYSQL_ROOT_PASSWORD";
} else {
$restoreCommand .= " < {$tmpPath}";
}
break;
case \App\Models\StandalonePostgresql::class:
$restoreCommand = $this->postgresqlRestoreCommand;
if ($this->dumpAll) {
$restoreCommand .= " && (gunzip -cf {$tmpPath} 2>/dev/null || cat {$tmpPath}) | psql -U \$POSTGRES_USER postgres";
} else {
$restoreCommand .= " {$tmpPath}";
}
break;
case \App\Models\StandaloneMongodb::class:
$restoreCommand = $this->mongodbRestoreCommand;
if ($this->dumpAll === false) {
$restoreCommand .= "{$tmpPath}";
}
break;
}
$restoreCommand = $this->buildRestoreCommand($tmpPath);
$restoreCommandBase64 = base64_encode($restoreCommand);
$this->importCommands[] = "echo \"{$restoreCommandBase64}\" | base64 -d > {$scriptPath}";
@ -311,9 +381,26 @@ EOD;
return;
}
// Clean the path (remove leading slash if present)
$cleanPath = ltrim($this->s3Path, '/');
// Validate the S3 path early to prevent command injection in subsequent operations
if (! $this->validateS3Path($cleanPath)) {
$this->dispatch('error', 'Invalid S3 path. Path must contain only safe characters (alphanumerics, dots, dashes, underscores, slashes).');
return;
}
try {
$s3Storage = S3Storage::ownedByCurrentTeam()->findOrFail($this->s3StorageId);
// Validate bucket name early
if (! $this->validateBucketName($s3Storage->bucket)) {
$this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only alphanumerics, dots, dashes, and underscores.');
return;
}
// Test connection
$s3Storage->testConnection();
@ -328,9 +415,6 @@ EOD;
'use_path_style_endpoint' => true,
]);
// Clean the path (remove leading slash if present)
$cleanPath = ltrim($this->s3Path, '/');
// Check if file exists
if (! $disk->exists($cleanPath)) {
$this->dispatch('error', 'File not found in S3. Please check the path.');
@ -375,9 +459,23 @@ EOD;
$bucket = $s3Storage->bucket;
$endpoint = $s3Storage->endpoint;
// Validate bucket name to prevent command injection
if (! $this->validateBucketName($bucket)) {
$this->dispatch('error', 'Invalid S3 bucket name. Bucket name must contain only alphanumerics, dots, dashes, and underscores.');
return;
}
// Clean the S3 path
$cleanPath = ltrim($this->s3Path, '/');
// Validate the S3 path to prevent command injection
if (! $this->validateS3Path($cleanPath)) {
$this->dispatch('error', 'Invalid S3 path. Path must contain only safe characters (alphanumerics, dots, dashes, underscores, slashes).');
return;
}
// Get helper image
$helperImage = config('constants.coolify.helper_image');
$latestVersion = getHelperVersion();
@ -410,11 +508,15 @@ EOD;
$escapedSecret = escapeshellarg($secret);
$commands[] = "docker exec {$containerName} mc alias set s3temp {$escapedEndpoint} {$escapedKey} {$escapedSecret}";
// 4. Check file exists in S3
$commands[] = "docker exec {$containerName} mc stat s3temp/{$bucket}/{$cleanPath}";
// 4. Check file exists in S3 (bucket and path already validated above)
$escapedBucket = escapeshellarg($bucket);
$escapedCleanPath = escapeshellarg($cleanPath);
$escapedS3Source = escapeshellarg("s3temp/{$bucket}/{$cleanPath}");
$commands[] = "docker exec {$containerName} mc stat {$escapedS3Source}";
// 5. Download from S3 to helper container (progress shown by default)
$commands[] = "docker exec {$containerName} mc cp s3temp/{$bucket}/{$cleanPath} {$helperTmpPath}";
$escapedHelperTmpPath = escapeshellarg($helperTmpPath);
$commands[] = "docker exec {$containerName} mc cp {$escapedS3Source} {$escapedHelperTmpPath}";
// 6. Copy from helper to server, then immediately to database container
$commands[] = "docker cp {$containerName}:{$helperTmpPath} {$serverTmpPath}";

View File

@ -129,7 +129,7 @@ class Form extends Component
$this->storage->refresh();
$this->isUsable = $this->storage->is_usable;
$this->dispatch('error', 'Failed to create storage.', $e->getMessage());
$this->dispatch('error', 'Failed to test connection.', $e->getMessage());
}
}

View File

@ -0,0 +1,25 @@
<?php
namespace App\Policies;
use App\Models\InstanceSettings;
use App\Models\User;
class InstanceSettingsPolicy
{
/**
* Determine whether the user can view the instance settings.
*/
public function view(User $user, InstanceSettings $settings): bool
{
return isInstanceAdmin();
}
/**
* Determine whether the user can update the instance settings.
*/
public function update(User $user, InstanceSettings $settings): bool
{
return isInstanceAdmin();
}
}

View File

@ -50,6 +50,9 @@ class AuthServiceProvider extends ServiceProvider
// API Token policy
\Laravel\Sanctum\PersonalAccessToken::class => \App\Policies\ApiTokenPolicy::class,
// Instance settings policy
\App\Models\InstanceSettings::class => \App\Policies\InstanceSettingsPolicy::class,
// Team policy
\App\Models\Team::class => \App\Policies\TeamPolicy::class,

View File

@ -210,7 +210,7 @@
wire:loading.attr="disabled"
type="{{ $type }}"
@disabled($disabled)
@if ($htmlId !== 'null') id={{ $htmlId }} @endif
@if ($htmlId !== 'null') id="{{ $htmlId }}" @endif
name="{{ $name }}"
placeholder="{{ $attributes->get('placeholder') }}"
@if ($autofocus) autofocus @endif>

View File

@ -148,7 +148,7 @@
<div x-show="filename && !error" class="pt-6">
<h3>File Information</h3>
<div class="pt-2">Location: <span x-text="filename ?? 'N/A'"></span> <span x-text="filesize">/ </span></div>
<div class="pt-2">Location: <span x-text="filename ?? 'N/A'"></span><span x-show="filesize" x-text="' / ' + filesize"></span></div>
<div class="pt-2">
<x-modal-confirmation title="Restore Database from File?" buttonTitle="Restore from File"
submitAction="runImport" isErrorButton>

View File

@ -9,7 +9,7 @@
<form wire:submit='submit' class="flex flex-col">
<div class="flex items-center gap-2">
<h2>General</h2>
<x-forms.button type="submit">
<x-forms.button canGate="update" :canResource="$settings" type="submit">
Save
</x-forms.button>
</div>
@ -18,10 +18,10 @@
<div class="flex flex-col gap-2">
<div class="flex flex-wrap items-end gap-2">
<div class="flex gap-2 md:flex-row flex-col w-full">
<x-forms.input id="fqdn" label="Domain"
<x-forms.input canGate="update" :canResource="$settings" id="fqdn" label="Domain"
helper="Enter the full domain name (FQDN) of the instance, including 'https://' if you want to secure the dashboard with HTTPS. Setting this will make the dashboard accessible via this domain, secured by HTTPS, instead of just the IP address."
placeholder="https://coolify.yourdomain.com" />
<x-forms.input id="instance_name" label="Name" placeholder="Coolify"
<x-forms.input canGate="update" :canResource="$settings" id="instance_name" label="Name" placeholder="Coolify"
helper="Custom name for your Coolify instance, shown in the URL." />
<div class="w-full" x-data="{
open: false,
@ -71,10 +71,10 @@
</div>
</div>
<div class="flex gap-2 md:flex-row flex-col w-full">
<x-forms.input id="public_ipv4" type="password" label="Instance's Public IPv4"
<x-forms.input canGate="update" :canResource="$settings" id="public_ipv4" type="password" label="Instance's Public IPv4"
helper="Enter the IPv4 address of the instance.<br><br>It is useful if you have several IPv4 addresses and Coolify could not detect the correct one."
placeholder="1.2.3.4" autocomplete="new-password" />
<x-forms.input id="public_ipv6" type="password" label="Instance's Public IPv6"
<x-forms.input canGate="update" :canResource="$settings" id="public_ipv6" type="password" label="Instance's Public IPv6"
helper="Enter the IPv6 address of the instance.<br><br>It is useful if you have several IPv6 addresses and Coolify could not detect the correct one."
placeholder="2001:db8::1" autocomplete="new-password" />
</div>
@ -86,11 +86,9 @@
</div>
@endif
@if(isDev())
<x-forms.input id="dev_helper_version" label="Dev Helper Version (Development Only)"
helper="Override the default coolify-helper image version. Leave empty to use the default version from config ({{ config('constants.coolify.helper_version') }}). Examples: 1.0.11, latest, dev"
placeholder="{{ config('constants.coolify.helper_version') }}" />
</div>
<x-forms.input canGate="update" :canResource="$settings" id="dev_helper_version" label="Dev Helper Version (Development Only)"
helper="Override the default coolify-helper image version. Leave empty to use the default version from config ({{ config('constants.coolify.helper_version') }}). Examples: 1.0.11, latest, dev"
placeholder="{{ config('constants.coolify.helper_version') }}" />
@endif
</div>
</form>

View File

@ -37,3 +37,92 @@ test('customLocation can be cleared to allow uploaded file to be used', function
expect($component->customLocation)->toBe('');
});
test('validateBucketName accepts valid bucket names', function () {
$component = new Import;
$method = new ReflectionMethod($component, 'validateBucketName');
// Valid bucket names
expect($method->invoke($component, 'my-bucket'))->toBeTrue();
expect($method->invoke($component, 'my_bucket'))->toBeTrue();
expect($method->invoke($component, 'mybucket123'))->toBeTrue();
expect($method->invoke($component, 'my.bucket.name'))->toBeTrue();
expect($method->invoke($component, 'Bucket-Name_123'))->toBeTrue();
});
test('validateBucketName rejects invalid bucket names', function () {
$component = new Import;
$method = new ReflectionMethod($component, 'validateBucketName');
// Invalid bucket names (command injection attempts)
expect($method->invoke($component, 'bucket;rm -rf /'))->toBeFalse();
expect($method->invoke($component, 'bucket$(whoami)'))->toBeFalse();
expect($method->invoke($component, 'bucket`id`'))->toBeFalse();
expect($method->invoke($component, 'bucket|cat /etc/passwd'))->toBeFalse();
expect($method->invoke($component, 'bucket&ls'))->toBeFalse();
expect($method->invoke($component, "bucket\nid"))->toBeFalse();
expect($method->invoke($component, 'bucket name'))->toBeFalse(); // Space not allowed in bucket
});
test('validateS3Path accepts valid S3 paths', function () {
$component = new Import;
$method = new ReflectionMethod($component, 'validateS3Path');
// Valid S3 paths
expect($method->invoke($component, 'backup.sql'))->toBeTrue();
expect($method->invoke($component, 'folder/backup.sql'))->toBeTrue();
expect($method->invoke($component, 'my-folder/my_backup.sql.gz'))->toBeTrue();
expect($method->invoke($component, 'path/to/deep/file.tar.gz'))->toBeTrue();
expect($method->invoke($component, 'folder with space/file.sql'))->toBeTrue();
});
test('validateS3Path rejects invalid S3 paths', function () {
$component = new Import;
$method = new ReflectionMethod($component, 'validateS3Path');
// Invalid S3 paths (command injection attempts)
expect($method->invoke($component, ''))->toBeFalse(); // Empty
expect($method->invoke($component, '../etc/passwd'))->toBeFalse(); // Directory traversal
expect($method->invoke($component, 'path;rm -rf /'))->toBeFalse();
expect($method->invoke($component, 'path$(whoami)'))->toBeFalse();
expect($method->invoke($component, 'path`id`'))->toBeFalse();
expect($method->invoke($component, 'path|cat /etc/passwd'))->toBeFalse();
expect($method->invoke($component, 'path&ls'))->toBeFalse();
expect($method->invoke($component, "path\nid"))->toBeFalse();
expect($method->invoke($component, "path\r\nid"))->toBeFalse();
expect($method->invoke($component, "path\0id"))->toBeFalse(); // Null byte
expect($method->invoke($component, "path'injection"))->toBeFalse();
expect($method->invoke($component, 'path"injection'))->toBeFalse();
expect($method->invoke($component, 'path\\injection'))->toBeFalse();
});
test('validateServerPath accepts valid server paths', function () {
$component = new Import;
$method = new ReflectionMethod($component, 'validateServerPath');
// Valid server paths (must be absolute)
expect($method->invoke($component, '/tmp/backup.sql'))->toBeTrue();
expect($method->invoke($component, '/var/backups/my-backup.sql'))->toBeTrue();
expect($method->invoke($component, '/home/user/data_backup.sql.gz'))->toBeTrue();
expect($method->invoke($component, '/path/to/deep/nested/file.tar.gz'))->toBeTrue();
});
test('validateServerPath rejects invalid server paths', function () {
$component = new Import;
$method = new ReflectionMethod($component, 'validateServerPath');
// Invalid server paths
expect($method->invoke($component, 'relative/path.sql'))->toBeFalse(); // Not absolute
expect($method->invoke($component, '/path/../etc/passwd'))->toBeFalse(); // Directory traversal
expect($method->invoke($component, '/path;rm -rf /'))->toBeFalse();
expect($method->invoke($component, '/path$(whoami)'))->toBeFalse();
expect($method->invoke($component, '/path`id`'))->toBeFalse();
expect($method->invoke($component, '/path|cat /etc/passwd'))->toBeFalse();
expect($method->invoke($component, '/path&ls'))->toBeFalse();
expect($method->invoke($component, "/path\nid"))->toBeFalse();
expect($method->invoke($component, "/path\r\nid"))->toBeFalse();
expect($method->invoke($component, "/path\0id"))->toBeFalse(); // Null byte
expect($method->invoke($component, "/path'injection"))->toBeFalse();
expect($method->invoke($component, '/path"injection'))->toBeFalse();
expect($method->invoke($component, '/path\\injection'))->toBeFalse();
});