fix: harden GetLogs Livewire component with locked properties and input validation
Add #[Locked] attributes to security-sensitive properties (resource, servicesubtype, server, container) to prevent client-side modification via Livewire wire protocol. Add container name validation using ValidationPatterns::isValidContainerName() and server ownership authorization via Server::ownedByCurrentTeam() in both getLogs() and downloadAllLogs() methods. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
e39678aea5
commit
48ba4ece3c
2 changed files with 137 additions and 5 deletions
|
|
@ -16,7 +16,9 @@
|
|||
use App\Models\StandaloneMysql;
|
||||
use App\Models\StandalonePostgresql;
|
||||
use App\Models\StandaloneRedis;
|
||||
use App\Support\ValidationPatterns;
|
||||
use Illuminate\Support\Facades\Process;
|
||||
use Livewire\Attributes\Locked;
|
||||
use Livewire\Component;
|
||||
|
||||
class GetLogs extends Component
|
||||
|
|
@ -29,12 +31,16 @@ class GetLogs extends Component
|
|||
|
||||
public string $errors = '';
|
||||
|
||||
#[Locked]
|
||||
public Application|Service|StandalonePostgresql|StandaloneRedis|StandaloneMongodb|StandaloneMysql|StandaloneMariadb|StandaloneKeydb|StandaloneDragonfly|StandaloneClickhouse|null $resource = null;
|
||||
|
||||
#[Locked]
|
||||
public ServiceApplication|ServiceDatabase|null $servicesubtype = null;
|
||||
|
||||
#[Locked]
|
||||
public Server $server;
|
||||
|
||||
#[Locked]
|
||||
public ?string $container = null;
|
||||
|
||||
public ?string $displayName = null;
|
||||
|
|
@ -54,7 +60,7 @@ class GetLogs extends Component
|
|||
public function mount()
|
||||
{
|
||||
if (! is_null($this->resource)) {
|
||||
if ($this->resource->getMorphClass() === \App\Models\Application::class) {
|
||||
if ($this->resource->getMorphClass() === Application::class) {
|
||||
$this->showTimeStamps = $this->resource->settings->is_include_timestamps;
|
||||
} else {
|
||||
if ($this->servicesubtype) {
|
||||
|
|
@ -63,7 +69,7 @@ public function mount()
|
|||
$this->showTimeStamps = $this->resource->is_include_timestamps;
|
||||
}
|
||||
}
|
||||
if ($this->resource?->getMorphClass() === \App\Models\Application::class) {
|
||||
if ($this->resource?->getMorphClass() === Application::class) {
|
||||
if (str($this->container)->contains('-pr-')) {
|
||||
$this->pull_request = 'Pull Request: '.str($this->container)->afterLast('-pr-')->beforeLast('_')->value();
|
||||
}
|
||||
|
|
@ -74,11 +80,11 @@ public function mount()
|
|||
public function instantSave()
|
||||
{
|
||||
if (! is_null($this->resource)) {
|
||||
if ($this->resource->getMorphClass() === \App\Models\Application::class) {
|
||||
if ($this->resource->getMorphClass() === Application::class) {
|
||||
$this->resource->settings->is_include_timestamps = $this->showTimeStamps;
|
||||
$this->resource->settings->save();
|
||||
}
|
||||
if ($this->resource->getMorphClass() === \App\Models\Service::class) {
|
||||
if ($this->resource->getMorphClass() === Service::class) {
|
||||
$serviceName = str($this->container)->beforeLast('-')->value();
|
||||
$subType = $this->resource->applications()->where('name', $serviceName)->first();
|
||||
if ($subType) {
|
||||
|
|
@ -118,10 +124,20 @@ public function toggleStreamLogs()
|
|||
|
||||
public function getLogs($refresh = false)
|
||||
{
|
||||
if (! Server::ownedByCurrentTeam()->where('id', $this->server->id)->exists()) {
|
||||
$this->outputs = 'Unauthorized.';
|
||||
|
||||
return;
|
||||
}
|
||||
if (! $this->server->isFunctional()) {
|
||||
return;
|
||||
}
|
||||
if (! $refresh && ! $this->expandByDefault && ($this->resource?->getMorphClass() === \App\Models\Service::class || str($this->container)->contains('-pr-'))) {
|
||||
if ($this->container && ! ValidationPatterns::isValidContainerName($this->container)) {
|
||||
$this->outputs = 'Invalid container name.';
|
||||
|
||||
return;
|
||||
}
|
||||
if (! $refresh && ! $this->expandByDefault && ($this->resource?->getMorphClass() === Service::class || str($this->container)->contains('-pr-'))) {
|
||||
return;
|
||||
}
|
||||
if ($this->numberOfLines <= 0 || is_null($this->numberOfLines)) {
|
||||
|
|
@ -194,9 +210,15 @@ public function copyLogs(): string
|
|||
|
||||
public function downloadAllLogs(): string
|
||||
{
|
||||
if (! Server::ownedByCurrentTeam()->where('id', $this->server->id)->exists()) {
|
||||
return '';
|
||||
}
|
||||
if (! $this->server->isFunctional() || ! $this->container) {
|
||||
return '';
|
||||
}
|
||||
if (! ValidationPatterns::isValidContainerName($this->container)) {
|
||||
return '';
|
||||
}
|
||||
|
||||
if ($this->showTimeStamps) {
|
||||
if ($this->server->isSwarm()) {
|
||||
|
|
|
|||
110
tests/Feature/GetLogsCommandInjectionTest.php
Normal file
110
tests/Feature/GetLogsCommandInjectionTest.php
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
<?php
|
||||
|
||||
use App\Livewire\Project\Shared\GetLogs;
|
||||
use App\Support\ValidationPatterns;
|
||||
use Livewire\Attributes\Locked;
|
||||
|
||||
describe('GetLogs locked properties', function () {
|
||||
test('container property has Locked attribute', function () {
|
||||
$property = new ReflectionProperty(GetLogs::class, 'container');
|
||||
$attributes = $property->getAttributes(Locked::class);
|
||||
|
||||
expect($attributes)->not->toBeEmpty();
|
||||
});
|
||||
|
||||
test('server property has Locked attribute', function () {
|
||||
$property = new ReflectionProperty(GetLogs::class, 'server');
|
||||
$attributes = $property->getAttributes(Locked::class);
|
||||
|
||||
expect($attributes)->not->toBeEmpty();
|
||||
});
|
||||
|
||||
test('resource property has Locked attribute', function () {
|
||||
$property = new ReflectionProperty(GetLogs::class, 'resource');
|
||||
$attributes = $property->getAttributes(Locked::class);
|
||||
|
||||
expect($attributes)->not->toBeEmpty();
|
||||
});
|
||||
|
||||
test('servicesubtype property has Locked attribute', function () {
|
||||
$property = new ReflectionProperty(GetLogs::class, 'servicesubtype');
|
||||
$attributes = $property->getAttributes(Locked::class);
|
||||
|
||||
expect($attributes)->not->toBeEmpty();
|
||||
});
|
||||
});
|
||||
|
||||
describe('GetLogs container name validation in getLogs', function () {
|
||||
test('getLogs method validates container name with ValidationPatterns', function () {
|
||||
$method = new ReflectionMethod(GetLogs::class, 'getLogs');
|
||||
$startLine = $method->getStartLine();
|
||||
$endLine = $method->getEndLine();
|
||||
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
|
||||
$methodBody = implode('', $lines);
|
||||
|
||||
expect($methodBody)->toContain('ValidationPatterns::isValidContainerName');
|
||||
});
|
||||
|
||||
test('downloadAllLogs method validates container name with ValidationPatterns', function () {
|
||||
$method = new ReflectionMethod(GetLogs::class, 'downloadAllLogs');
|
||||
$startLine = $method->getStartLine();
|
||||
$endLine = $method->getEndLine();
|
||||
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
|
||||
$methodBody = implode('', $lines);
|
||||
|
||||
expect($methodBody)->toContain('ValidationPatterns::isValidContainerName');
|
||||
});
|
||||
});
|
||||
|
||||
describe('GetLogs authorization checks', function () {
|
||||
test('getLogs method checks server ownership via ownedByCurrentTeam', function () {
|
||||
$method = new ReflectionMethod(GetLogs::class, 'getLogs');
|
||||
$startLine = $method->getStartLine();
|
||||
$endLine = $method->getEndLine();
|
||||
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
|
||||
$methodBody = implode('', $lines);
|
||||
|
||||
expect($methodBody)->toContain('Server::ownedByCurrentTeam()');
|
||||
});
|
||||
|
||||
test('downloadAllLogs method checks server ownership via ownedByCurrentTeam', function () {
|
||||
$method = new ReflectionMethod(GetLogs::class, 'downloadAllLogs');
|
||||
$startLine = $method->getStartLine();
|
||||
$endLine = $method->getEndLine();
|
||||
$lines = array_slice(file($method->getFileName()), $startLine - 1, $endLine - $startLine + 1);
|
||||
$methodBody = implode('', $lines);
|
||||
|
||||
expect($methodBody)->toContain('Server::ownedByCurrentTeam()');
|
||||
});
|
||||
});
|
||||
|
||||
describe('GetLogs container name injection payloads are blocked by validation', function () {
|
||||
test('newline injection payload is rejected', function () {
|
||||
// The exact PoC payload from the advisory
|
||||
$payload = "postgresql 2>/dev/null\necho '===RCE-START==='\nid\nwhoami\nhostname\ncat /etc/hostname\necho '===RCE-END==='\n#";
|
||||
expect(ValidationPatterns::isValidContainerName($payload))->toBeFalse();
|
||||
});
|
||||
|
||||
test('semicolon injection payload is rejected', function () {
|
||||
expect(ValidationPatterns::isValidContainerName('postgresql;id'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('backtick injection payload is rejected', function () {
|
||||
expect(ValidationPatterns::isValidContainerName('postgresql`id`'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('command substitution injection payload is rejected', function () {
|
||||
expect(ValidationPatterns::isValidContainerName('postgresql$(whoami)'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('pipe injection payload is rejected', function () {
|
||||
expect(ValidationPatterns::isValidContainerName('postgresql|cat /etc/passwd'))->toBeFalse();
|
||||
});
|
||||
|
||||
test('valid container names are accepted', function () {
|
||||
expect(ValidationPatterns::isValidContainerName('postgresql'))->toBeTrue();
|
||||
expect(ValidationPatterns::isValidContainerName('my-app-container'))->toBeTrue();
|
||||
expect(ValidationPatterns::isValidContainerName('service_db.v2'))->toBeTrue();
|
||||
expect(ValidationPatterns::isValidContainerName('coolify-proxy'))->toBeTrue();
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue