Merge branch 'ghsa-mw5w-2vvh-mgf4-investigation'
This commit is contained in:
commit
d174724bf6
8 changed files with 165 additions and 6 deletions
|
|
@ -2196,7 +2196,7 @@ private function clone_repository()
|
||||||
$this->create_workdir();
|
$this->create_workdir();
|
||||||
$this->execute_remote_command(
|
$this->execute_remote_command(
|
||||||
[
|
[
|
||||||
executeInDocker($this->deployment_uuid, "cd {$this->workdir} && git log -1 {$this->commit} --pretty=%B"),
|
executeInDocker($this->deployment_uuid, "cd {$this->workdir} && git log -1 ".escapeshellarg($this->commit)." --pretty=%B"),
|
||||||
'hidden' => true,
|
'hidden' => true,
|
||||||
'save' => 'commit_message',
|
'save' => 'commit_message',
|
||||||
]
|
]
|
||||||
|
|
|
||||||
|
|
@ -37,7 +37,7 @@ class General extends Component
|
||||||
#[Validate(['required'])]
|
#[Validate(['required'])]
|
||||||
public string $gitBranch;
|
public string $gitBranch;
|
||||||
|
|
||||||
#[Validate(['string', 'nullable'])]
|
#[Validate(['string', 'nullable', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'])]
|
||||||
public ?string $gitCommitSha = null;
|
public ?string $gitCommitSha = null;
|
||||||
|
|
||||||
#[Validate(['string', 'nullable'])]
|
#[Validate(['string', 'nullable'])]
|
||||||
|
|
@ -184,7 +184,7 @@ protected function rules(): array
|
||||||
'fqdn' => 'nullable',
|
'fqdn' => 'nullable',
|
||||||
'gitRepository' => 'required',
|
'gitRepository' => 'required',
|
||||||
'gitBranch' => 'required',
|
'gitBranch' => 'required',
|
||||||
'gitCommitSha' => 'nullable',
|
'gitCommitSha' => ['nullable', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'],
|
||||||
'installCommand' => 'nullable',
|
'installCommand' => 'nullable',
|
||||||
'buildCommand' => 'nullable',
|
'buildCommand' => 'nullable',
|
||||||
'startCommand' => 'nullable',
|
'startCommand' => 'nullable',
|
||||||
|
|
|
||||||
|
|
@ -50,6 +50,8 @@ public function rollbackImage($commit)
|
||||||
{
|
{
|
||||||
$this->authorize('deploy', $this->application);
|
$this->authorize('deploy', $this->application);
|
||||||
|
|
||||||
|
$commit = validateGitRef($commit, 'rollback commit');
|
||||||
|
|
||||||
$deployment_uuid = new Cuid2;
|
$deployment_uuid = new Cuid2;
|
||||||
|
|
||||||
$result = queue_application_deployment(
|
$result = queue_application_deployment(
|
||||||
|
|
|
||||||
|
|
@ -30,7 +30,7 @@ class Source extends Component
|
||||||
#[Validate(['required', 'string'])]
|
#[Validate(['required', 'string'])]
|
||||||
public string $gitBranch;
|
public string $gitBranch;
|
||||||
|
|
||||||
#[Validate(['nullable', 'string'])]
|
#[Validate(['nullable', 'string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'])]
|
||||||
public ?string $gitCommitSha = null;
|
public ?string $gitCommitSha = null;
|
||||||
|
|
||||||
#[Locked]
|
#[Locked]
|
||||||
|
|
|
||||||
|
|
@ -1686,7 +1686,8 @@ public function fqdns(): Attribute
|
||||||
|
|
||||||
protected function buildGitCheckoutCommand($target): string
|
protected function buildGitCheckoutCommand($target): string
|
||||||
{
|
{
|
||||||
$command = "git checkout $target";
|
$escapedTarget = escapeshellarg($target);
|
||||||
|
$command = "git checkout {$escapedTarget}";
|
||||||
|
|
||||||
if ($this->settings->is_git_submodules_enabled) {
|
if ($this->settings->is_git_submodules_enabled) {
|
||||||
$command .= ' && git submodule update --init --recursive';
|
$command .= ' && git submodule update --init --recursive';
|
||||||
|
|
|
||||||
|
|
@ -92,7 +92,7 @@ function sharedDataApplications()
|
||||||
'static_image' => Rule::enum(StaticImageTypes::class),
|
'static_image' => Rule::enum(StaticImageTypes::class),
|
||||||
'domains' => 'string|nullable',
|
'domains' => 'string|nullable',
|
||||||
'redirect' => Rule::enum(RedirectTypes::class),
|
'redirect' => Rule::enum(RedirectTypes::class),
|
||||||
'git_commit_sha' => 'string',
|
'git_commit_sha' => ['string', 'regex:/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/'],
|
||||||
'docker_registry_image_name' => 'string|nullable',
|
'docker_registry_image_name' => 'string|nullable',
|
||||||
'docker_registry_image_tag' => 'string|nullable',
|
'docker_registry_image_tag' => 'string|nullable',
|
||||||
'install_command' => 'string|nullable',
|
'install_command' => 'string|nullable',
|
||||||
|
|
|
||||||
|
|
@ -147,6 +147,39 @@ function validateShellSafePath(string $input, string $context = 'path'): string
|
||||||
return $input;
|
return $input;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validate that a string is a safe git ref (commit SHA, branch name, tag, or HEAD).
|
||||||
|
*
|
||||||
|
* Prevents command injection by enforcing an allowlist of characters valid for git refs.
|
||||||
|
* Valid: hex SHAs, HEAD, branch/tag names (alphanumeric, dots, hyphens, underscores, slashes).
|
||||||
|
*
|
||||||
|
* @param string $input The git ref to validate
|
||||||
|
* @param string $context Descriptive name for error messages
|
||||||
|
* @return string The validated input (trimmed)
|
||||||
|
*
|
||||||
|
* @throws \Exception If the input contains disallowed characters
|
||||||
|
*/
|
||||||
|
function validateGitRef(string $input, string $context = 'git ref'): string
|
||||||
|
{
|
||||||
|
$input = trim($input);
|
||||||
|
|
||||||
|
if ($input === '' || $input === 'HEAD') {
|
||||||
|
return $input;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Must not start with a hyphen (git flag injection)
|
||||||
|
if (str_starts_with($input, '-')) {
|
||||||
|
throw new \Exception("Invalid {$context}: must not start with a hyphen.");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Allow only alphanumeric characters, dots, hyphens, underscores, and slashes
|
||||||
|
if (! preg_match('/^[a-zA-Z0-9][a-zA-Z0-9._\-\/]*$/', $input)) {
|
||||||
|
throw new \Exception("Invalid {$context}: contains disallowed characters. Only alphanumeric characters, dots, hyphens, underscores, and slashes are allowed.");
|
||||||
|
}
|
||||||
|
|
||||||
|
return $input;
|
||||||
|
}
|
||||||
|
|
||||||
function generate_readme_file(string $name, string $updated_at): string
|
function generate_readme_file(string $name, string $updated_at): string
|
||||||
{
|
{
|
||||||
$name = sanitize_string($name);
|
$name = sanitize_string($name);
|
||||||
|
|
|
||||||
123
tests/Unit/GitRefValidationTest.php
Normal file
123
tests/Unit/GitRefValidationTest.php
Normal file
|
|
@ -0,0 +1,123 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Security tests for git ref validation (GHSA-mw5w-2vvh-mgf4).
|
||||||
|
*
|
||||||
|
* Ensures that git_commit_sha and related inputs are validated
|
||||||
|
* to prevent OS command injection via shell metacharacters.
|
||||||
|
*/
|
||||||
|
|
||||||
|
describe('validateGitRef', function () {
|
||||||
|
test('accepts valid hex commit SHAs', function () {
|
||||||
|
expect(validateGitRef('abc123def456'))->toBe('abc123def456');
|
||||||
|
expect(validateGitRef('a3e59e5c9'))->toBe('a3e59e5c9');
|
||||||
|
expect(validateGitRef('abc123def456abc123def456abc123def456abc123'))->toBe('abc123def456abc123def456abc123def456abc123');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('accepts HEAD', function () {
|
||||||
|
expect(validateGitRef('HEAD'))->toBe('HEAD');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('accepts empty string', function () {
|
||||||
|
expect(validateGitRef(''))->toBe('');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('accepts branch and tag names', function () {
|
||||||
|
expect(validateGitRef('main'))->toBe('main');
|
||||||
|
expect(validateGitRef('feature/my-branch'))->toBe('feature/my-branch');
|
||||||
|
expect(validateGitRef('v1.2.3'))->toBe('v1.2.3');
|
||||||
|
expect(validateGitRef('release-2.0'))->toBe('release-2.0');
|
||||||
|
expect(validateGitRef('my_branch'))->toBe('my_branch');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('trims whitespace', function () {
|
||||||
|
expect(validateGitRef(' abc123 '))->toBe('abc123');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects single quote injection', function () {
|
||||||
|
expect(fn () => validateGitRef("HEAD'; id >/tmp/poc; #"))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects semicolon command separator', function () {
|
||||||
|
expect(fn () => validateGitRef('abc123; rm -rf /'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects command substitution with $()', function () {
|
||||||
|
expect(fn () => validateGitRef('$(whoami)'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects backtick command substitution', function () {
|
||||||
|
expect(fn () => validateGitRef('`whoami`'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects pipe operator', function () {
|
||||||
|
expect(fn () => validateGitRef('abc | cat /etc/passwd'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects ampersand operator', function () {
|
||||||
|
expect(fn () => validateGitRef('abc & whoami'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects hash comment injection', function () {
|
||||||
|
expect(fn () => validateGitRef('abc #'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects newline injection', function () {
|
||||||
|
expect(fn () => validateGitRef("abc\nwhoami"))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects redirect operators', function () {
|
||||||
|
expect(fn () => validateGitRef('abc > /tmp/out'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects hyphen-prefixed input (git flag injection)', function () {
|
||||||
|
expect(fn () => validateGitRef('--upload-pack=malicious'))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects the exact PoC payload from advisory', function () {
|
||||||
|
expect(fn () => validateGitRef("HEAD'; whoami >/tmp/coolify_poc_git; #"))
|
||||||
|
->toThrow(Exception::class);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('executeInDocker git log escaping', function () {
|
||||||
|
test('git log command escapes commit SHA to prevent injection', function () {
|
||||||
|
$maliciousCommit = "HEAD'; id; #";
|
||||||
|
$command = "cd /workdir && git log -1 ".escapeshellarg($maliciousCommit).' --pretty=%B';
|
||||||
|
$result = executeInDocker('test-container', $command);
|
||||||
|
|
||||||
|
// The malicious payload must not be able to break out of quoting
|
||||||
|
expect($result)->not->toContain("id;");
|
||||||
|
expect($result)->toContain("'HEAD'\\''");
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('buildGitCheckoutCommand escaping', function () {
|
||||||
|
test('checkout command escapes target to prevent injection', function () {
|
||||||
|
$app = new \App\Models\Application;
|
||||||
|
$app->forceFill(['uuid' => 'test-uuid']);
|
||||||
|
|
||||||
|
$settings = new \App\Models\ApplicationSetting;
|
||||||
|
$settings->is_git_submodules_enabled = false;
|
||||||
|
$app->setRelation('settings', $settings);
|
||||||
|
|
||||||
|
$method = new \ReflectionMethod($app, 'buildGitCheckoutCommand');
|
||||||
|
|
||||||
|
$result = $method->invoke($app, 'abc123');
|
||||||
|
expect($result)->toContain("git checkout 'abc123'");
|
||||||
|
|
||||||
|
$result = $method->invoke($app, "abc'; id; #");
|
||||||
|
expect($result)->not->toContain("id;");
|
||||||
|
expect($result)->toContain("git checkout 'abc'");
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue