fix(ssh): escape scp source and destination
Quote SCP operands when building commands to prevent shell injection through source or destination paths, and cover the escaping behavior in the SSH command injection tests.
This commit is contained in:
parent
a22a0c027d
commit
ebf23f4874
2 changed files with 17 additions and 5 deletions
|
|
@ -63,10 +63,10 @@ public static function generateScpCommand(Server $server, string $source, string
|
|||
$scpCommand .= self::getCommonSshOptions($server, $sshKeyLocation, self::getConnectionTimeout($server), config('constants.ssh.server_interval'), isScp: true);
|
||||
|
||||
if ($server->isIpv6()) {
|
||||
return $scpCommand."{$source} ".escapeshellarg($server->user).'@['.escapeshellarg($server->ip)."]:{$dest}";
|
||||
return $scpCommand.escapeshellarg($source).' '.escapeshellarg($server->user).'@['.escapeshellarg($server->ip).']:'.escapeshellarg($dest);
|
||||
}
|
||||
|
||||
return $scpCommand."{$source} ".self::escapedUserAtHost($server).":{$dest}";
|
||||
return $scpCommand.escapeshellarg($source).' '.self::escapedUserAtHost($server).':'.escapeshellarg($dest);
|
||||
}
|
||||
|
||||
public static function generateSshCommand(Server $server, string $command, bool $disableMultiplexing = false): string
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
<?php
|
||||
|
||||
use App\Helpers\SshMultiplexingHelper;
|
||||
use App\Models\Server;
|
||||
use App\Rules\ValidHostname;
|
||||
use App\Rules\ValidServerIp;
|
||||
|
||||
|
|
@ -57,20 +58,20 @@
|
|||
// -------------------------------------------------------------------------
|
||||
|
||||
it('strips dangerous characters from server ip on write', function () {
|
||||
$server = new App\Models\Server;
|
||||
$server = new 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 = new 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 = new Server;
|
||||
$server->port = '22; evil';
|
||||
expect($server->port)->toBe(22);
|
||||
});
|
||||
|
|
@ -102,6 +103,17 @@
|
|||
expect($source)->not->toContain('{$server->user}@{$server->ip}');
|
||||
});
|
||||
|
||||
it('escapes scp source and destination operands', function () {
|
||||
$reflection = new ReflectionClass(SshMultiplexingHelper::class);
|
||||
$source = file_get_contents($reflection->getFileName());
|
||||
|
||||
expect($source)
|
||||
->toContain('escapeshellarg($source)')
|
||||
->toContain('escapeshellarg($dest)')
|
||||
->not->toContain('"{$source} "')
|
||||
->not->toContain('":{$dest}"');
|
||||
});
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// ValidHostname rejects shell metacharacters
|
||||
// -------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in a new issue