rosslh/coolify
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(ssh): prevent RCE via SSH command injection (#8748)

Browse source
This commit is contained in:
Andras Bacsai 2026-03-03 11:52:50 +01:00 committed by GitHub
commit fad1b95578
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 202 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
app/Helpers/SshMultiplexingHelper.php
View file

@ -37,7 +37,7 @@ public static function ensureMultiplexedConnection(Server $server): bool
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
$checkCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
}
$checkCommand .= "{$server->user}@{$server->ip}";
$checkCommand .= self::escapedUserAtHost($server);
$process = Process::run($checkCommand);
if ($process->exitCode() !== 0) {
@ -80,7 +80,7 @@ public static function establishNewMultiplexedConnection(Server $server): bool
$establishCommand .= ' -o ProxyCommand="cloudflared access ssh --hostname %h" ';
}
$establishCommand .= self::getCommonSshOptions($server, $sshKeyLocation, $connectionTimeout, $serverInterval);
$establishCommand .= "{$server->user}@{$server->ip}";
$establishCommand .= self::escapedUserAtHost($server);
$establishProcess = Process::run($establishCommand);
if ($establishProcess->exitCode() !== 0) {
return false;
@ -101,7 +101,7 @@ public static function removeMuxFile(Server $server)
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
$closeCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
}
$closeCommand .= "{$server->user}@{$server->ip}";
$closeCommand .= self::escapedUserAtHost($server);
Process::run($closeCommand);
// Clear connection metadata from cache
@ -141,9 +141,9 @@ public static function generateScpCommand(Server $server, string $source, string
$scp_command .= self::getCommonSshOptions($server, $sshKeyLocation, config('constants.ssh.connection_timeout'), config('constants.ssh.server_interval'), isScp: true);
if ($server->isIpv6()) {
$scp_command .= "{$source} {$server->user}@[{$server->ip}]:{$dest}";
$scp_command .= "{$source} ".escapeshellarg($server->user).'@['.escapeshellarg($server->ip)."]:{$dest}";
} else {
$scp_command .= "{$source} {$server->user}@{$server->ip}:{$dest}";
$scp_command .= "{$source} ".self::escapedUserAtHost($server).":{$dest}";
}
return $scp_command;
@ -189,13 +189,18 @@ public static function generateSshCommand(Server $server, string $command, bool
$delimiter = base64_encode($delimiter);
$command = str_replace($delimiter, '', $command);
$ssh_command .= "{$server->user}@{$server->ip} 'bash -se' << \\$delimiter".PHP_EOL
$ssh_command .= self::escapedUserAtHost($server)." 'bash -se' << \\$delimiter".PHP_EOL
.$command.PHP_EOL
.$delimiter;
return $ssh_command;
}
private static function escapedUserAtHost(Server $server): string
{
return escapeshellarg($server->user).'@'.escapeshellarg($server->ip);
}
private static function isMultiplexingEnabled(): bool
{
return config('constants.ssh.mux_enabled') && ! config('constants.coolify.is_windows_docker_desktop');
@ -224,9 +229,9 @@ private static function getCommonSshOptions(Server $server, string $sshKeyLocati
// Bruh
if ($isScp) {
$options .= "-P {$server->port} ";
$options .= '-P '.escapeshellarg((string) $server->port).' ';
} else {
$options .= "-p {$server->port} ";
$options .= '-p '.escapeshellarg((string) $server->port).' ';
}
return $options;
@ -245,7 +250,7 @@ public static function isConnectionHealthy(Server $server): bool
if (data_get($server, 'settings.is_cloudflare_tunnel')) {
$healthCommand .= '-o ProxyCommand="cloudflared access ssh --hostname %h" ';
}
$healthCommand .= "{$server->user}@{$server->ip} 'echo \"health_check_ok\"'";
$healthCommand .= self::escapedUserAtHost($server)." 'echo \"health_check_ok\"'";
$process = Process::run($healthCommand);
$isHealthy = $process->exitCode() === 0 && str_contains($process->output(), 'health_check_ok');

13
app/Http/Controllers/Api/ServersController.php
View file

@ -11,6 +11,7 @@
use App\Models\PrivateKey;
use App\Models\Project;
use App\Models\Server as ModelsServer;
use App\Rules\ValidServerIp;
use Illuminate\Http\Request;
use OpenApi\Attributes as OA;
use Stringable;
@ -472,10 +473,10 @@ public function create_server(Request $request)
$validator = customApiValidator($request->all(), [
'name' => 'string|max:255',
'description' => 'string|nullable',
'ip' => 'string|required',
'port' => 'integer|nullable',
'ip' => ['string', 'required', new ValidServerIp],
'port' => 'integer|nullable|between:1,65535',
'private_key_uuid' => 'string|required',
'user' => 'string|nullable',
'user' => ['string', 'nullable', 'regex:/^[a-zA-Z0-9_-]+$/'],
'is_build_server' => 'boolean|nullable',
'instant_validate' => 'boolean|nullable',
'proxy_type' => 'string|nullable',
@ -637,10 +638,10 @@ public function update_server(Request $request)
$validator = customApiValidator($request->all(), [
'name' => 'string|max:255|nullable',
'description' => 'string|nullable',
'ip' => 'string|nullable',
'port' => 'integer|nullable',
'ip' => ['string', 'nullable', new ValidServerIp],
'port' => 'integer|nullable|between:1,65535',
'private_key_uuid' => 'string|nullable',
'user' => 'string|nullable',
'user' => ['string', 'nullable', 'regex:/^[a-zA-Z0-9_-]+$/'],
'is_build_server' => 'boolean|nullable',
'instant_validate' => 'boolean|nullable',
'proxy_type' => 'string|nullable',

5
app/Livewire/Server/New/ByIp.php
View file

@ -5,6 +5,7 @@
use App\Enums\ProxyTypes;
use App\Models\Server;
use App\Models\Team;
use App\Rules\ValidServerIp;
use App\Support\ValidationPatterns;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
use Livewire\Attributes\Locked;
@ -55,8 +56,8 @@ protected function rules(): array
'new_private_key_value' => 'nullable|string',
'name' => ValidationPatterns::nameRules(),
'description' => ValidationPatterns::descriptionRules(),
'ip' => 'required|string',
'user' => 'required|string',
'ip' => ['required', 'string', new ValidServerIp],
'user' => ['required', 'string', 'regex:/^[a-zA-Z0-9_-]+$/'],
'port' => 'required|integer|between:1,65535',
'is_build_server' => 'required|boolean',
];

7
app/Livewire/Server/Show.php
View file

@ -7,6 +7,7 @@
use App\Events\ServerReachabilityChanged;
use App\Models\CloudProviderToken;
use App\Models\Server;
use App\Rules\ValidServerIp;
use App\Services\HetznerService;
use App\Support\ValidationPatterns;
use Illuminate\Foundation\Auth\Access\AuthorizesRequests;
@ -106,9 +107,9 @@ protected function rules(): array
return [
'name' => ValidationPatterns::nameRules(),
'description' => ValidationPatterns::descriptionRules(),
'ip' => 'required',
'user' => 'required',
'port' => 'required',
'ip' => ['required', new ValidServerIp],
'user' => ['required', 'regex:/^[a-zA-Z0-9_-]+$/'],
'port' => 'required|integer|between:1,65535',
'validationLogs' => 'nullable',
'wildcardDomain' => 'nullable|url',
'isReachable' => 'required',

9
app/Models/Server.php
View file

@ -913,6 +913,9 @@ public function port(): Attribute
return Attribute::make(
get: function ($value) {
return (int) preg_replace('/[^0-9]/', '', $value);
},
set: function ($value) {
return (int) preg_replace('/[^0-9]/', '', (string) $value);
}
);
}
@ -922,6 +925,9 @@ public function user(): Attribute
return Attribute::make(
get: function ($value) {
return preg_replace('/[^A-Za-z0-9\-_]/', '', $value);
},
set: function ($value) {
return preg_replace('/[^A-Za-z0-9\-_]/', '', $value);
}
);
}
@ -931,6 +937,9 @@ public function ip(): Attribute
return Attribute::make(
get: function ($value) {
return preg_replace('/[^0-9a-zA-Z.:%-]/', '', $value);
},
set: function ($value) {
return preg_replace('/[^0-9a-zA-Z.:%-]/', '', $value);
}
);
}

40
app/Rules/ValidServerIp.php Normal file
View file

@ -0,0 +1,40 @@
<?php
namespace App\Rules;
use Closure;
use Illuminate\Contracts\Validation\ValidationRule;
class ValidServerIp implements ValidationRule
{
/**
* Accepts a valid IPv4 address, IPv6 address, or RFC 1123 hostname.
*/
public function validate(string $attribute, mixed $value, Closure $fail): void
{
if (empty($value)) {
return;
}
$trimmed = trim($value);
if (filter_var($trimmed, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
return;
}
if (filter_var($trimmed, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
return;
}
// Delegate hostname validation to ValidHostname
$hostnameRule = new ValidHostname;
$failed = false;
$hostnameRule->validate($attribute, $trimmed, function () use (&$failed) {
$failed = true;
});
if ($failed) {
$fail('The :attribute must be a valid IPv4 address, IPv6 address, or hostname.');
}
}
}

125
tests/Unit/SshCommandInjectionTest.php Normal file
View file

@ -0,0 +1,125 @@
<?php
use App\Helpers\SshMultiplexingHelper;
use App\Rules\ValidHostname;
use App\Rules\ValidServerIp;
// -------------------------------------------------------------------------
// ValidServerIp rule
// -------------------------------------------------------------------------
it('accepts a valid IPv4 address', function () {
$rule = new ValidServerIp;
$failed = false;
$rule->validate('ip', '192.168.1.1', function () use (&$failed) {
$failed = true;
});
expect($failed)->toBeFalse();
});
it('accepts a valid IPv6 address', function () {
$rule = new ValidServerIp;
$failed = false;
$rule->validate('ip', '2001:db8::1', function () use (&$failed) {
$failed = true;
});
expect($failed)->toBeFalse();
});
it('accepts a valid hostname', function () {
$rule = new ValidServerIp;
$failed = false;
$rule->validate('ip', 'my-server.example.com', function () use (&$failed) {
$failed = true;
});
expect($failed)->toBeFalse();
});
it('rejects injection payloads in server ip', function (string $payload) {
$rule = new ValidServerIp;
$failed = false;
$rule->validate('ip', $payload, function () use (&$failed) {
$failed = true;
});
expect($failed)->toBeTrue("ValidServerIp should reject: $payload");
})->with([
'semicolon' => ['192.168.1.1; rm -rf /'],
'pipe' => ['192.168.1.1 | cat /etc/passwd'],
'backtick' => ['192.168.1.1`id`'],
'dollar subshell' => ['192.168.1.1$(id)'],
'ampersand' => ['192.168.1.1 & id'],
'newline' => ["192.168.1.1\nid"],
'null byte' => ["192.168.1.1\0id"],
]);
// -------------------------------------------------------------------------
// Server model setter casts
// -------------------------------------------------------------------------
it('strips dangerous characters from server ip on write', function () {
$server = new App\Models\Server;
$server->ip = '192.168.1.1;rm -rf /';
// Regex [^0-9a-zA-Z.:%-] removes ; space and /; hyphen is allowed
expect($server->ip)->toBe('192.168.1.1rm-rf');
});
it('strips dangerous characters from server user on write', function () {
$server = new App\Models\Server;
$server->user = 'root$(id)';
expect($server->user)->toBe('rootid');
});
it('strips non-numeric characters from server port on write', function () {
$server = new App\Models\Server;
$server->port = '22; evil';
expect($server->port)->toBe(22);
});
// -------------------------------------------------------------------------
// escapeshellarg() in generated SSH commands (source-level verification)
// -------------------------------------------------------------------------
it('has escapedUserAtHost private static helper in SshMultiplexingHelper', function () {
$reflection = new ReflectionClass(SshMultiplexingHelper::class);
expect($reflection->hasMethod('escapedUserAtHost'))->toBeTrue();
$method = $reflection->getMethod('escapedUserAtHost');
expect($method->isPrivate())->toBeTrue();
expect($method->isStatic())->toBeTrue();
});
it('wraps port with escapeshellarg in getCommonSshOptions', function () {
$reflection = new ReflectionClass(SshMultiplexingHelper::class);
$source = file_get_contents($reflection->getFileName());
expect($source)->toContain('escapeshellarg((string) $server->port)');
});
it('has no raw user@ip string interpolation in SshMultiplexingHelper', function () {
$reflection = new ReflectionClass(SshMultiplexingHelper::class);
$source = file_get_contents($reflection->getFileName());
expect($source)->not->toContain('{$server->user}@{$server->ip}');
});
// -------------------------------------------------------------------------
// ValidHostname rejects shell metacharacters
// -------------------------------------------------------------------------
it('rejects semicolon in hostname', function () {
$rule = new ValidHostname;
$failed = false;
$rule->validate('hostname', 'example.com;id', function () use (&$failed) {
$failed = true;
});
expect($failed)->toBeTrue();
});
it('rejects backtick in hostname', function () {
$rule = new ValidHostname;
$failed = false;
$rule->validate('hostname', 'example.com`id`', function () use (&$failed) {
$failed = true;
});
expect($failed)->toBeTrue();
});