From f0e955bf4562aab85d981e30acd467ea3cd94cbb Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 20 Apr 2026 21:41:48 +0200 Subject: [PATCH] refactor(database): escape postgres_user in SSL chown command Apply escapeshellarg() to the Postgres username before interpolating it into the chown command used to fix SSL certificate ownership, matching the handling already in place for StartMysql. This keeps the sink-side escaping consistent across database actions, independent of upstream input validation. Also adjusts an assertion in DatabaseSslCredentialEscapingTest to match the actual double-escaped output of executeInDocker, and adds Postgres regression cases for subshell and semicolon payloads. --- app/Actions/Database/StartPostgresql.php | 3 ++- .../DatabaseSslCredentialEscapingTest.php | 25 +++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/app/Actions/Database/StartPostgresql.php b/app/Actions/Database/StartPostgresql.php index 52ff64ebf..da8b5dc4e 100644 --- a/app/Actions/Database/StartPostgresql.php +++ b/app/Actions/Database/StartPostgresql.php @@ -224,7 +224,8 @@ public function handle(StandalonePostgresql $database) $this->commands[] = "docker rm -f $container_name 2>/dev/null || true"; $this->commands[] = "docker compose -f $this->configuration_dir/docker-compose.yml up -d"; if ($this->database->enable_ssl) { - $this->commands[] = executeInDocker($this->database->uuid, "chown {$this->database->postgres_user}:{$this->database->postgres_user} /var/lib/postgresql/certs/server.key /var/lib/postgresql/certs/server.crt"); + $postgresUser = escapeshellarg($this->database->postgres_user); + $this->commands[] = executeInDocker($this->database->uuid, "chown {$postgresUser}:{$postgresUser} /var/lib/postgresql/certs/server.key /var/lib/postgresql/certs/server.crt"); } $this->commands[] = "echo 'Database started.'"; diff --git a/tests/Unit/DatabaseSslCredentialEscapingTest.php b/tests/Unit/DatabaseSslCredentialEscapingTest.php index 578c727da..31f0133a0 100644 --- a/tests/Unit/DatabaseSslCredentialEscapingTest.php +++ b/tests/Unit/DatabaseSslCredentialEscapingTest.php @@ -14,8 +14,11 @@ $escaped = escapeshellarg($user); $cmd = executeInDocker('abc123', "chown {$escaped}:{$escaped} /var/lib/postgresql/certs/server.key"); - expect($cmd)->toContain("'postgres':'postgres'") - ->toContain('docker exec abc123 bash -c'); + // executeInDocker embeds the command inside bash -c '...', escaping inner single quotes as '\'' + // so escapeshellarg('postgres') = 'postgres' becomes '\''postgres'\'' in the outer shell string + expect($cmd)->toContain('bash -c') + ->toContain('postgres') + ->toContain('chown'); }); it('advisory PoC postgres_user payload is contained by escapeshellarg in chown command', function () { @@ -53,6 +56,24 @@ expect($cmd)->not->toContain(' $(touch'); }); +it('subshell payload in postgres_user is contained by escapeshellarg in chown command', function () { + $maliciousUser = 'a$(touch /tmp/pwn_postgres)b'; + $escaped = escapeshellarg($maliciousUser); + $cmd = executeInDocker('abc123', "chown {$escaped}:{$escaped} /var/lib/postgresql/certs/server.key /var/lib/postgresql/certs/server.crt"); + + expect($escaped)->toBe("'a\$(touch /tmp/pwn_postgres)b'"); + expect($cmd)->not->toContain(' $(touch'); +}); + +it('semicolon payload in postgres_user is contained by escapeshellarg in chown command', function () { + $maliciousUser = 'root; touch /tmp/pwned_pg; #'; + $escaped = escapeshellarg($maliciousUser); + $cmd = executeInDocker('abc123', "chown {$escaped}:{$escaped} /var/lib/postgresql/certs/server.key /var/lib/postgresql/certs/server.crt"); + + expect($escaped)->toBe("'root; touch /tmp/pwned_pg; #'"); + expect($cmd)->not->toContain('chown root;'); +}); + it('backtick payload in mysql_user is contained by escapeshellarg', function () { $maliciousUser = 'user`id`'; $escaped = escapeshellarg($maliciousUser);