fix(security): Enhance file upload security and cleanup processes for terminal file upload

This commit is contained in:
Ahliman HUSEYNOV 2025-11-13 11:21:21 +01:00
parent 72020a7363
commit 903621dbaa
No known key found for this signature in database
4 changed files with 33 additions and 11 deletions

View file

@ -6,6 +6,7 @@ use Illuminate\Http\Request;
use Illuminate\Http\UploadedFile; use Illuminate\Http\UploadedFile;
use Illuminate\Routing\Controller as BaseController; use Illuminate\Routing\Controller as BaseController;
use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Auth;
use Illuminate\Support\Str;
use Pion\Laravel\ChunkUpload\Exceptions\UploadMissingFileException; use Pion\Laravel\ChunkUpload\Exceptions\UploadMissingFileException;
use Pion\Laravel\ChunkUpload\Handler\HandlerFactory; use Pion\Laravel\ChunkUpload\Handler\HandlerFactory;
use Pion\Laravel\ChunkUpload\Receiver\FileReceiver; use Pion\Laravel\ChunkUpload\Receiver\FileReceiver;
@ -114,13 +115,28 @@ class UploadController extends BaseController
mkdir($finalPath, 0755, true); mkdir($finalPath, 0755, true);
} }
// Use original filename with timestamp to avoid conflicts // Security: Generate safe filename server-side to prevent path traversal
$filename = time().'_'.$file->getClientOriginalName(); $originalName = $file->getClientOriginalName();
$file->move($finalPath, $filename); $extension = $file->getClientOriginalExtension();
// Create a safe slug from original filename (without extension)
$nameWithoutExt = pathinfo($originalName, PATHINFO_FILENAME);
$safeSlug = Str::slug($nameWithoutExt); // Converts to lowercase, replaces special chars with dashes
$safeSlug = substr($safeSlug, 0, 50); // Limit length
// Sanitize extension (only allow alphanumeric)
$safeExtension = preg_replace('/[^a-zA-Z0-9]/', '', $extension);
// Generate safe filename: timestamp_slug_randomhash.ext
$randomHash = Str::random(16);
$safeFilename = time().'_'.$safeSlug.'_'.$randomHash.($safeExtension ? '.'.$safeExtension : '');
$file->move($finalPath, $safeFilename);
return response()->json([ return response()->json([
'mime_type' => $mime, 'mime_type' => $mime,
'filename' => $filename, 'filename' => $safeFilename,
'original_name' => $originalName, // Keep original name for reference
]); ]);
} }
} }

View file

@ -41,9 +41,10 @@ class CleanupExpiredTerminalFilesJob implements ShouldQueue
// Delete file from server // Delete file from server
$server = Server::find($this->serverId); $server = Server::find($this->serverId);
if ($server) { if ($server) {
// Remove from server // Remove from server - escape shell arguments to prevent injection
$escapedServerPath = escapeshellarg($this->serverPath);
$result = instant_remote_process([ $result = instant_remote_process([
"rm -f {$this->serverPath}" "rm -f {$escapedServerPath}"
], $server, throwError: false); ], $server, throwError: false);
if ($result) { if ($result) {
@ -52,9 +53,12 @@ class CleanupExpiredTerminalFilesJob implements ShouldQueue
// If container was specified, remove from container as well // If container was specified, remove from container as well
if ($this->containerUuid) { if ($this->containerUuid) {
$containerPath = "/tmp/{$this->filename}"; $escapedContainerUuid = escapeshellarg($this->containerUuid);
$escapedFilename = escapeshellarg($this->filename);
$containerPath = "/tmp/{$this->filename}"; // For logging only
instant_remote_process([ instant_remote_process([
"docker exec {$this->containerUuid} rm -f {$containerPath} 2>/dev/null || true" "docker exec {$escapedContainerUuid} rm -f /tmp/{$escapedFilename} 2>/dev/null || true"
], $server, throwError: false); ], $server, throwError: false);
Log::info("Cleaned up container terminal file: {$containerPath}"); Log::info("Cleaned up container terminal file: {$containerPath}");

View file

@ -130,14 +130,17 @@ class FileImport extends Component
// Copy file to server's temporary directory // Copy file to server's temporary directory
$serverTmpPath = "/tmp/coolify_import_{$uploadId}_{$sanitizedFilename}"; $serverTmpPath = "/tmp/coolify_import_{$uploadId}_{$sanitizedFilename}";
instant_scp($finalPath, $serverTmpPath, $server); $safeServerTmpPath = escapeshellarg($serverTmpPath);
instant_scp($finalPath, $safeServerTmpPath, $server);
// If it's a container, copy to container // If it's a container, copy to container
if ($isContainer) { if ($isContainer) {
$containerPath = "/tmp/{$sanitizedFilename}"; $containerPath = "/tmp/{$sanitizedFilename}";
$safeContainer = escapeshellarg($this->selectedUuid);
$safeContainerPath = escapeshellarg($containerPath);
instant_remote_process([ instant_remote_process([
"docker cp {$serverTmpPath} {$this->selectedUuid}:{$containerPath}", "docker cp {$safeServerTmpPath} {$safeContainer}:{$safeContainerPath}",
], $server); ], $server);
$this->filePath = $containerPath; $this->filePath = $containerPath;

View file

@ -1,7 +1,6 @@
<div x-data="{ error: $wire.entangle('error'), filesize: $wire.entangle('filesize'), filename: $wire.entangle('filename'), isUploading: $wire.entangle('isUploading'), progress: $wire.entangle('progress'), filePath: $wire.entangle('filePath') }"> <div x-data="{ error: $wire.entangle('error'), filesize: $wire.entangle('filesize'), filename: $wire.entangle('filename'), isUploading: $wire.entangle('isUploading'), progress: $wire.entangle('progress'), filePath: $wire.entangle('filePath') }">
<div class="pb-4"> <div class="pb-4">
<h2>Import File for Terminal</h2>
<div class="text-sm text-neutral-500 pb-2"> <div class="text-sm text-neutral-500 pb-2">
Upload a file that will be temporarily stored and accessible in your selected server or container. Upload a file that will be temporarily stored and accessible in your selected server or container.
Perfect for importing SQL dumps, configuration files, or any other data. Perfect for importing SQL dumps, configuration files, or any other data.