Skip to content

Commit 9e96a20

Browse files
andrasbacsaiandrasbacsai
authored
fix: add validation and escaping for Docker network names (#9228)
2 parents 1027c73 + 3d1b9f5 commit 9e96a20

12 files changed

Lines changed: 211 additions & 34 deletions

File tree

‎app/Actions/Service/StartService.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ public function handle(Service $service, bool $pullLatestImages = false, bool $s
4040
$commands[] = "docker network connect $service->uuid coolify-proxy >/dev/null 2>&1 || true";
4141
if (data_get($service, 'connect_to_docker_network')) {
4242
$compose = data_get($service, 'docker_compose', []);
43-
$network = $service->destination->network;
43+
$safeNetwork = escapeshellarg($service->destination->network);
4444
$serviceNames = data_get(Yaml::parse($compose), 'services', []);
4545
foreach ($serviceNames as $serviceName => $serviceConfig) {
46-
$commands[] = "docker network connect --alias {$serviceName}-{$service->uuid} $network {$serviceName}-{$service->uuid} >/dev/null 2>&1 || true";
46+
$commands[] = "docker network connect --alias {$serviceName}-{$service->uuid} {$safeNetwork} {$serviceName}-{$service->uuid} >/dev/null 2>&1 || true";
4747
}
4848
}
4949

‎app/Console/Commands/Init.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,18 +212,19 @@ private function cleanupUnusedNetworkFromCoolifyProxy()
212212
$removeNetworks = $allNetworks->diff($networks);
213213
$commands = collect();
214214
foreach ($removeNetworks as $network) {
215-
$out = instant_remote_process(["docker network inspect -f json $network | jq '.[].Containers | if . == {} then null else . end'"], $server, false);
215+
$safe = escapeshellarg($network);
216+
$out = instant_remote_process(["docker network inspect -f json {$safe} | jq '.[].Containers | if . == {} then null else . end'"], $server, false);
216217
if (empty($out)) {
217-
$commands->push("docker network disconnect $network coolify-proxy >/dev/null 2>&1 || true");
218-
$commands->push("docker network rm $network >/dev/null 2>&1 || true");
218+
$commands->push("docker network disconnect {$safe} coolify-proxy >/dev/null 2>&1 || true");
219+
$commands->push("docker network rm {$safe} >/dev/null 2>&1 || true");
219220
} else {
220221
$data = collect(json_decode($out, true));
221222
if ($data->count() === 1) {
222223
// If only coolify-proxy itself is connected to that network (it should not be possible, but who knows)
223224
$isCoolifyProxyItself = data_get($data->first(), 'Name') === 'coolify-proxy';
224225
if ($isCoolifyProxyItself) {
225-
$commands->push("docker network disconnect $network coolify-proxy >/dev/null 2>&1 || true");
226-
$commands->push("docker network rm $network >/dev/null 2>&1 || true");
226+
$commands->push("docker network disconnect {$safe} coolify-proxy >/dev/null 2>&1 || true");
227+
$commands->push("docker network rm {$safe} >/dev/null 2>&1 || true");
227228
}
228229
}
229230
}

‎app/Jobs/ApplicationDeploymentJob.php‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,8 @@ public function handle(): void
288288
// Make sure the private key is stored in the filesystem
289289
$this->server->privateKey->storeInFileSystem();
290290
// Generate custom host<->ip mapping
291-
$allContainers = instant_remote_process(["docker network inspect {$this->destination->network} -f '{{json .Containers}}' "], $this->server);
291+
$safeNetwork = escapeshellarg($this->destination->network);
292+
$allContainers = instant_remote_process(["docker network inspect {$safeNetwork} -f '{{json .Containers}}' "], $this->server);
292293

293294
if (! is_null($allContainers)) {
294295
$allContainers = format_docker_command_output_to_json($allContainers);
@@ -2015,9 +2016,11 @@ private function prepare_builder_image(bool $firstTry = true)
20152016
$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}";
20162017
} else {
20172018
if ($this->dockerConfigFileExists === 'OK') {
2018-
$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}";
2019+
$safeNetwork = escapeshellarg($this->destination->network);
2020+
$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}";
20192021
} else {
2020-
$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}";
2022+
$safeNetwork = escapeshellarg($this->destination->network);
2023+
$runCommand = "docker run -d --network {$safeNetwork} --name {$this->deployment_uuid} {$env_flags} --rm -v /var/run/docker.sock:/var/run/docker.sock {$helperImage}";
20212024
}
20222025
}
20232026
if ($firstTry) {
@@ -3046,28 +3049,29 @@ private function build_image()
30463049
$this->execute_remote_command([executeInDocker($this->deployment_uuid, 'rm '.self::NIXPACKS_PLAN_PATH), 'hidden' => true]);
30473050
} else {
30483051
// Dockerfile buildpack
3052+
$safeNetwork = escapeshellarg($this->destination->network);
30493053
if ($this->dockerSecretsSupported) {
30503054
// Modify the Dockerfile to use build secrets
30513055
$this->modify_dockerfile_for_secrets("{$this->workdir}{$this->dockerfile_location}");
30523056
$secrets_flags = $this->build_secrets ? " {$this->build_secrets}" : '';
30533057
if ($this->force_rebuild) {
3054-
$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}");
3058+
$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}");
30553059
} else {
3056-
$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}");
3060+
$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}");
30573061
}
30583062
} elseif ($this->dockerBuildkitSupported) {
30593063
// BuildKit without secrets
30603064
if ($this->force_rebuild) {
3061-
$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}");
3065+
$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}");
30623066
} else {
3063-
$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}");
3067+
$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}");
30643068
}
30653069
} else {
30663070
// Traditional build with args
30673071
if ($this->force_rebuild) {
3068-
$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}");
3072+
$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}");
30693073
} else {
3070-
$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}");
3074+
$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}");
30713075
}
30723076
}
30733077
$base64_build_command = base64_encode($build_command);

‎app/Jobs/DatabaseBackupJob.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,7 @@ private function upload_to_s3(): void
678678
} else {
679679
$network = $this->database->destination->network;
680680
}
681+
$safeNetwork = escapeshellarg($network);
681682

682683
$fullImageName = $this->getFullImageName();
683684

@@ -689,13 +690,13 @@ private function upload_to_s3(): void
689690
if (isDev()) {
690691
if ($this->database->name === 'coolify-db') {
691692
$backup_location_from = '/var/lib/docker/volumes/coolify_dev_backups_data/_data/coolify/coolify-db-'.$this->server->ip.$this->backup_file;
692-
$commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}";
693+
$commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}";
693694
} else {
694695
$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;
695-
$commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}";
696+
$commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $backup_location_from:$this->backup_location:ro {$fullImageName}";
696697
}
697698
} else {
698-
$commands[] = "docker run -d --network {$network} --name backup-of-{$this->backup_log_uuid} --rm -v $this->backup_location:$this->backup_location:ro {$fullImageName}";
699+
$commands[] = "docker run -d --network {$safeNetwork} --name backup-of-{$this->backup_log_uuid} --rm -v $this->backup_location:$this->backup_location:ro {$fullImageName}";
699700
}
700701

701702
// Escape S3 credentials to prevent command injection

‎app/Livewire/Destination/New/Docker.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use App\Models\Server;
66
use App\Models\StandaloneDocker;
77
use App\Models\SwarmDocker;
8+
use App\Support\ValidationPatterns;
89
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
910
use Livewire\Attributes\Locked;
1011
use Livewire\Attributes\Validate;
@@ -24,7 +25,7 @@ class Docker extends Component
2425
#[Validate(['required', 'string'])]
2526
public string $name;
2627

27-
#[Validate(['required', 'string'])]
28+
#[Validate(['required', 'string', 'max:255', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'])]
2829
public string $network;
2930

3031
#[Validate(['required', 'string'])]

‎app/Livewire/Destination/Show.php‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class Show extends Component
2020
#[Validate(['string', 'required'])]
2121
public string $name;
2222

23-
#[Validate(['string', 'required'])]
23+
#[Validate(['string', 'required', 'max:255', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/'])]
2424
public string $network;
2525

2626
#[Validate(['string', 'required'])]
@@ -84,8 +84,9 @@ public function delete()
8484
if ($this->destination->attachedTo()) {
8585
return $this->dispatch('error', 'You must delete all resources before deleting this destination.');
8686
}
87-
instant_remote_process(["docker network disconnect {$this->destination->network} coolify-proxy"], $this->destination->server, throwError: false);
88-
instant_remote_process(['docker network rm -f '.$this->destination->network], $this->destination->server);
87+
$safeNetwork = escapeshellarg($this->destination->network);
88+
instant_remote_process(["docker network disconnect {$safeNetwork} coolify-proxy"], $this->destination->server, throwError: false);
89+
instant_remote_process(["docker network rm -f {$safeNetwork}"], $this->destination->server);
8990
}
9091
$this->destination->delete();
9192

‎app/Models/StandaloneDocker.php‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
namespace App\Models;
44

55
use App\Jobs\ConnectProxyToNetworksJob;
6+
use App\Support\ValidationPatterns;
67
use App\Traits\HasSafeStringAttribute;
78
use Illuminate\Database\Eloquent\Factories\HasFactory;
89

@@ -18,13 +19,23 @@ protected static function boot()
1819
parent::boot();
1920
static::created(function ($newStandaloneDocker) {
2021
$server = $newStandaloneDocker->server;
22+
$safeNetwork = escapeshellarg($newStandaloneDocker->network);
2123
instant_remote_process([
22-
"docker network inspect $newStandaloneDocker->network >/dev/null 2>&1 || docker network create --driver overlay --attachable $newStandaloneDocker->network >/dev/null",
24+
"docker network inspect {$safeNetwork} >/dev/null 2>&1 || docker network create --driver overlay --attachable {$safeNetwork} >/dev/null",
2325
], $server, false);
2426
ConnectProxyToNetworksJob::dispatchSync($server);
2527
});
2628
}
2729

30+
public function setNetworkAttribute(string $value): void
31+
{
32+
if (! ValidationPatterns::isValidDockerNetwork($value)) {
33+
throw new \InvalidArgumentException('Invalid Docker network name. Must start with alphanumeric and contain only alphanumeric characters, dots, hyphens, and underscores.');
34+
}
35+
36+
$this->attributes['network'] = $value;
37+
}
38+
2839
public function applications()
2940
{
3041
return $this->morphMany(Application::class, 'destination');

‎app/Models/SwarmDocker.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,21 @@
22

33
namespace App\Models;
44

5+
use App\Support\ValidationPatterns;
6+
57
class SwarmDocker extends BaseModel
68
{
79
protected $guarded = [];
810

11+
public function setNetworkAttribute(string $value): void
12+
{
13+
if (! ValidationPatterns::isValidDockerNetwork($value)) {
14+
throw new \InvalidArgumentException('Invalid Docker network name. Must start with alphanumeric and contain only alphanumeric characters, dots, hyphens, and underscores.');
15+
}
16+
17+
$this->attributes['network'] = $value;
18+
}
19+
920
public function applications()
1021
{
1122
return $this->morphMany(Application::class, 'destination');

‎app/Support/ValidationPatterns.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ class ValidationPatterns
5858
*/
5959
public const CONTAINER_NAME_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/';
6060

61+
/**
62+
* Pattern for Docker network names
63+
* Must start with alphanumeric, followed by alphanumeric, dots, hyphens, or underscores
64+
* Matches Docker's network naming rules and prevents shell injection
65+
*/
66+
public const DOCKER_NETWORK_PATTERN = '/^[a-zA-Z0-9][a-zA-Z0-9._-]*$/';
67+
6168
/**
6269
* Get validation rules for name fields
6370
*/
@@ -210,6 +217,44 @@ public static function isValidContainerName(string $name): bool
210217
return preg_match(self::CONTAINER_NAME_PATTERN, $name) === 1;
211218
}
212219

220+
/**
221+
* Get validation rules for Docker network name fields
222+
*/
223+
public static function dockerNetworkRules(bool $required = true, int $maxLength = 255): array
224+
{
225+
$rules = [];
226+
227+
if ($required) {
228+
$rules[] = 'required';
229+
} else {
230+
$rules[] = 'nullable';
231+
}
232+
233+
$rules[] = 'string';
234+
$rules[] = "max:$maxLength";
235+
$rules[] = 'regex:'.self::DOCKER_NETWORK_PATTERN;
236+
237+
return $rules;
238+
}
239+
240+
/**
241+
* Get validation messages for Docker network name fields
242+
*/
243+
public static function dockerNetworkMessages(string $field = 'network'): array
244+
{
245+
return [
246+
"{$field}.regex" => 'The network name must start with an alphanumeric character and contain only alphanumeric characters, dots, hyphens, and underscores.',
247+
];
248+
}
249+
250+
/**
251+
* Check if a string is a valid Docker network name.
252+
*/
253+
public static function isValidDockerNetwork(string $name): bool
254+
{
255+
return preg_match(self::DOCKER_NETWORK_PATTERN, $name) === 1;
256+
}
257+
213258
/**
214259
* Get combined validation messages for both name and description fields
215260
*/

0 commit comments

Comments
 (0)