mirror of
https://github.com/tiennm99/coolify.git
synced 2026-10-14 08:18:08 +00:00
fix(database): enforce credential format validation and sanitize init/SSL arguments
Add ValidationPatterns helpers for database identifiers and passwords, apply them across database Livewire components and the API controller, encode MongoDB init script values via json_encode, and pass the MySQL user through escapeshellarg when generating SSL chown commands.
This commit is contained in:
1 parent
2264a2ef76
commit
03313e54cc
14 files changed
+502
-82
No files matched your search
@@ -0,0 +1,176 @@
|
||||
<?php
|
||||
|
||||
use App\Support\ValidationPatterns;
|
||||
use Illuminate\Support\Facades\Validator;
|
||||
|
||||
// ── DB_IDENTIFIER_PATTERN ─────────────────────────────────────────────────────
|
||||
|
||||
it('DB_IDENTIFIER_PATTERN accepts valid SQL identifiers', function (string $id) {
|
||||
expect(preg_match(ValidationPatterns::DB_IDENTIFIER_PATTERN, $id))->toBe(1);
|
||||
})->with([
|
||||
'simple lowercase' => 'postgres',
|
||||
'underscore prefix' => '_admin',
|
||||
'mixed case' => 'MyDatabase',
|
||||
'alphanumeric' => 'App_DB_1',
|
||||
'single char' => 'a',
|
||||
'all caps' => 'ROOT',
|
||||
'numbers in middle' => 'db2user',
|
||||
]);
|
||||
|
||||
it('DB_IDENTIFIER_PATTERN rejects shell-dangerous and invalid identifiers', function (string $id) {
|
||||
expect(preg_match(ValidationPatterns::DB_IDENTIFIER_PATTERN, $id))->toBe(0);
|
||||
})->with([
|
||||
'semicolon' => 'user;id',
|
||||
'pipe' => 'user|cat',
|
||||
'ampersand' => 'user&rm',
|
||||
'dollar sign' => 'user$x',
|
||||
'backtick' => 'user`id`',
|
||||
'subshell' => 'user$(id)',
|
||||
'space' => 'user name',
|
||||
'newline' => "user\nname",
|
||||
'single quote' => "user'name",
|
||||
'double quote' => 'user"name',
|
||||
'backslash' => 'user\\name',
|
||||
'less than' => 'user<name',
|
||||
'greater than' => 'user>name',
|
||||
'leading digit' => '1user',
|
||||
'hyphen' => 'my-user',
|
||||
'dot' => 'my.user',
|
||||
'empty' => '',
|
||||
'64 chars (over limit)' => str_repeat('a', 64),
|
||||
'advisory poc payload' => 'root; touch /tmp/pwned_rce; #',
|
||||
'subshell payload' => 'a$(touch /tmp/pwn)b',
|
||||
]);
|
||||
|
||||
// ── DB_PASSWORD_PATTERN ───────────────────────────────────────────────────────
|
||||
|
||||
it('DB_PASSWORD_PATTERN accepts strong passwords without shell-dangerous chars', function (string $pw) {
|
||||
expect(preg_match(ValidationPatterns::DB_PASSWORD_PATTERN, $pw))->toBe(1);
|
||||
})->with([
|
||||
'alphanumeric' => 'SecurePass123',
|
||||
'with special safe chars' => 'P@ss!word#1',
|
||||
'with brackets' => 'P{a}ss[word]',
|
||||
'with slash' => 'Pass/word1',
|
||||
'with dot comma' => 'Pass.word,1',
|
||||
'with hyphen' => 'Pass-word1',
|
||||
'with plus equals' => 'Pass+word=1',
|
||||
'with tilde colon' => 'P~ass:word1',
|
||||
'complex strong' => 'Str0ng!P@ss#word^123',
|
||||
]);
|
||||
|
||||
it('DB_PASSWORD_PATTERN rejects shell-dangerous characters', function (string $pw) {
|
||||
expect(preg_match(ValidationPatterns::DB_PASSWORD_PATTERN, $pw))->toBe(0);
|
||||
})->with([
|
||||
'backtick' => 'pass`word`',
|
||||
'dollar sign' => 'pass$word',
|
||||
'semicolon' => 'pass;word',
|
||||
'pipe' => 'pass|word',
|
||||
'ampersand' => 'pass&word',
|
||||
'less than' => 'pass<word',
|
||||
'greater than' => 'pass>word',
|
||||
'backslash' => 'pass\\word',
|
||||
'single quote' => "pass'word",
|
||||
'double quote' => 'pass"word',
|
||||
'space' => 'pass word',
|
||||
'newline' => "pass\nword",
|
||||
'carriage return' => "pass\rword",
|
||||
'tab' => "pass\tword",
|
||||
'empty' => '',
|
||||
'command substitution' => '$(whoami)',
|
||||
'rce payload' => 'root; touch /tmp/pwned; #',
|
||||
]);
|
||||
|
||||
// ── Rule helpers ──────────────────────────────────────────────────────────────
|
||||
|
||||
it('databaseIdentifierRules returns required by default', function () {
|
||||
$rules = ValidationPatterns::databaseIdentifierRules();
|
||||
|
||||
expect($rules)->toContain('required')
|
||||
->toContain('string')
|
||||
->toContain('min:1')
|
||||
->toContain('max:63')
|
||||
->toContain('regex:'.ValidationPatterns::DB_IDENTIFIER_PATTERN);
|
||||
});
|
||||
|
||||
it('databaseIdentifierRules returns nullable when not required', function () {
|
||||
$rules = ValidationPatterns::databaseIdentifierRules(required: false);
|
||||
|
||||
expect($rules)->toContain('nullable')
|
||||
->not->toContain('required');
|
||||
});
|
||||
|
||||
it('databasePasswordRules returns required by default', function () {
|
||||
$rules = ValidationPatterns::databasePasswordRules();
|
||||
|
||||
expect($rules)->toContain('required')
|
||||
->toContain('string')
|
||||
->toContain('min:1')
|
||||
->toContain('max:128')
|
||||
->toContain('regex:'.ValidationPatterns::DB_PASSWORD_PATTERN);
|
||||
});
|
||||
|
||||
it('databasePasswordRules returns nullable when not required', function () {
|
||||
$rules = ValidationPatterns::databasePasswordRules(required: false);
|
||||
|
||||
expect($rules)->toContain('nullable')
|
||||
->not->toContain('required');
|
||||
});
|
||||
|
||||
it('isValidDatabaseIdentifier returns true for valid identifier', function () {
|
||||
expect(ValidationPatterns::isValidDatabaseIdentifier('postgres'))->toBeTrue();
|
||||
expect(ValidationPatterns::isValidDatabaseIdentifier('_admin'))->toBeTrue();
|
||||
expect(ValidationPatterns::isValidDatabaseIdentifier('DB_1'))->toBeTrue();
|
||||
});
|
||||
|
||||
it('isValidDatabaseIdentifier returns false for injection payloads', function () {
|
||||
expect(ValidationPatterns::isValidDatabaseIdentifier('user; id'))->toBeFalse();
|
||||
expect(ValidationPatterns::isValidDatabaseIdentifier('user$(whoami)'))->toBeFalse();
|
||||
expect(ValidationPatterns::isValidDatabaseIdentifier(''))->toBeFalse();
|
||||
});
|
||||
|
||||
// ── Validator integration ─────────────────────────────────────────────────────
|
||||
|
||||
it('Laravel Validator rejects advisory PoC postgres_user payload', function () {
|
||||
$validator = Validator::make(
|
||||
['postgres_user' => 'root; touch /tmp/pwned_rce; #'],
|
||||
['postgres_user' => ValidationPatterns::databaseIdentifierRules()]
|
||||
);
|
||||
|
||||
expect($validator->fails())->toBeTrue();
|
||||
});
|
||||
|
||||
it('Laravel Validator rejects subshell injection in postgres_user', function () {
|
||||
$validator = Validator::make(
|
||||
['postgres_user' => 'a$(touch /tmp/pwn)b'],
|
||||
['postgres_user' => ValidationPatterns::databaseIdentifierRules()]
|
||||
);
|
||||
|
||||
expect($validator->fails())->toBeTrue();
|
||||
});
|
||||
|
||||
it('Laravel Validator accepts clean postgres_user', function () {
|
||||
$validator = Validator::make(
|
||||
['postgres_user' => 'postgres'],
|
||||
['postgres_user' => ValidationPatterns::databaseIdentifierRules()]
|
||||
);
|
||||
|
||||
expect($validator->fails())->toBeFalse();
|
||||
});
|
||||
|
||||
it('Laravel Validator rejects shell metachar in password', function () {
|
||||
$validator = Validator::make(
|
||||
['postgres_password' => 'pass$(id)word'],
|
||||
['postgres_password' => ValidationPatterns::databasePasswordRules()]
|
||||
);
|
||||
|
||||
expect($validator->fails())->toBeTrue();
|
||||
});
|
||||
|
||||
it('Laravel Validator accepts safe password', function () {
|
||||
$validator = Validator::make(
|
||||
['postgres_password' => 'Str0ng!P@ss#123'],
|
||||
['postgres_password' => ValidationPatterns::databasePasswordRules()]
|
||||
);
|
||||
|
||||
expect($validator->fails())->toBeFalse();
|
||||
});
|
||||
@@ -0,0 +1,149 @@
|
||||
<?php
|
||||
|
||||
/**
|
||||
* GHSA-rcch-8c74-7f29 — Sink-side escaping tests
|
||||
*
|
||||
* Verifies that credentials reaching shell commands are properly escaped
|
||||
* even if validation is bypassed (e.g. legacy rows, direct DB writes).
|
||||
*/
|
||||
|
||||
// ── executeInDocker + escapeshellarg chown pattern ────────────────────────────
|
||||
|
||||
it('escapeshellarg wraps postgres_user in single quotes for chown command', function () {
|
||||
$user = 'postgres';
|
||||
$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');
|
||||
});
|
||||
|
||||
it('advisory PoC postgres_user payload is contained by escapeshellarg in chown command', function () {
|
||||
// Simulates a legacy row that bypassed validation
|
||||
$maliciousUser = 'root; touch /tmp/pwned_rce; #';
|
||||
$escaped = escapeshellarg($maliciousUser);
|
||||
|
||||
// escapeshellarg must wrap the entire payload in single quotes
|
||||
// (semicolons inside single-quoted args are NOT shell metacharacters)
|
||||
expect($escaped)->toBe("'root; touch /tmp/pwned_rce; #'");
|
||||
|
||||
$cmd = executeInDocker('abc123', "chown {$escaped}:{$escaped} /var/lib/postgresql/certs/server.key");
|
||||
|
||||
// The cmd contains the payload, but ONLY inside single-quoted segments — cannot break out.
|
||||
// Verify the chown arg is never an unquoted bare ; — the payload is inside '...'
|
||||
// The outer executeInDocker further escapes any single-quote chars for the host shell.
|
||||
expect($cmd)->toContain('docker exec abc123 bash -c');
|
||||
|
||||
// Before fix: chown root; touch /tmp/pwned_rce; # ... (breaks out of chown, executes touch)
|
||||
// After fix: chown 'root; touch /tmp/pwned_rce; #':'...' ... (literal arg to chown)
|
||||
// The unescaped sequence "chown root;" must NOT appear.
|
||||
expect($cmd)->not->toContain('chown root;');
|
||||
});
|
||||
|
||||
it('subshell payload in mysql_user is contained by escapeshellarg in chown command', function () {
|
||||
$maliciousUser = 'a$(touch /tmp/pwn)b';
|
||||
$escaped = escapeshellarg($maliciousUser);
|
||||
$cmd = executeInDocker('abc123', "chown {$escaped}:{$escaped} /etc/mysql/certs/server.crt");
|
||||
|
||||
// escapeshellarg wraps in single quotes — $() is not expanded inside single quotes
|
||||
expect($escaped)->toBe("'a\$(touch /tmp/pwn)b'");
|
||||
|
||||
// The cmd must not contain an unquoted $( sequence — it must be inside single quotes
|
||||
// If the sequence appears at all, it must be single-quoted (the quote precedes it).
|
||||
expect($cmd)->not->toContain(' $(touch');
|
||||
});
|
||||
|
||||
it('backtick payload in mysql_user is contained by escapeshellarg', function () {
|
||||
$maliciousUser = 'user`id`';
|
||||
$escaped = escapeshellarg($maliciousUser);
|
||||
$cmd = executeInDocker('abc123', "chown {$escaped}:{$escaped} /etc/mysql/certs/server.crt");
|
||||
|
||||
// escapeshellarg wraps the whole value in single quotes — backticks not expanded inside ''
|
||||
expect($escaped)->toBe("'user`id`'");
|
||||
|
||||
// The unquoted bare backtick sequence `id` must not appear outside single-quoted context.
|
||||
// Specifically, "chown user`id`" (unquoted) must not appear.
|
||||
expect($cmd)->not->toContain('chown user`id`');
|
||||
});
|
||||
|
||||
// ── MongoDB JS init script JSON-escaping ──────────────────────────────────────
|
||||
|
||||
it('json_encode prevents JS injection in mongo_initdb_database', function () {
|
||||
$database = 'x"}); db.dropUser("admin"); //';
|
||||
$dbJson = json_encode($database, JSON_UNESCAPED_SLASHES);
|
||||
|
||||
// The double-quotes in the payload MUST be escaped — they cannot close the JS string literal.
|
||||
// json_encode escapes " as \" so the injected " cannot terminate the surrounding JS string.
|
||||
expect($dbJson)->toContain('\\"');
|
||||
|
||||
// The resulting JSON literal, when embedded in JS, forms a valid quoted string.
|
||||
// It starts and ends with the outermost " added by json_encode.
|
||||
expect($dbJson)->toStartWith('"')
|
||||
->toEndWith('"');
|
||||
|
||||
// Verify the injected payload is present but neutralised (the " that would close the JS
|
||||
// string is now escaped as \", preventing breakout).
|
||||
expect($dbJson)->toContain('x\\"});');
|
||||
});
|
||||
|
||||
it('json_encode prevents JS injection in mongo_initdb_root_username', function () {
|
||||
$username = 'admin", pwd: "", roles: [{role:"root", db:"admin"}]}); //';
|
||||
$userJson = json_encode($username, JSON_UNESCAPED_SLASHES);
|
||||
|
||||
$content = 'db.createUser({user: '.$userJson.', pwd: "secret", roles: []});';
|
||||
|
||||
// The injected " that would close the JS string must be escaped as \"
|
||||
expect($userJson)->toContain('\\"');
|
||||
|
||||
// The raw unescaped sequence admin" (with unescaped quote) must not appear in the JS
|
||||
expect($content)->not->toContain('admin", pwd');
|
||||
});
|
||||
|
||||
it('json_encode safely encodes a clean mongo username', function () {
|
||||
$username = 'mongouser';
|
||||
$userJson = json_encode($username, JSON_UNESCAPED_SLASHES);
|
||||
|
||||
expect($userJson)->toBe('"mongouser"');
|
||||
});
|
||||
|
||||
it('json_encode safely encodes a mongo password with special chars', function () {
|
||||
$password = 'P@ss!#word123';
|
||||
$pwdJson = json_encode($password, JSON_UNESCAPED_SLASHES);
|
||||
|
||||
expect($pwdJson)->toBe('"P@ss!#word123"');
|
||||
});
|
||||
|
||||
// ── Healthcheck CMD exec-form structure (no shell parsing) ────────────────────
|
||||
|
||||
it('CMD exec-form healthcheck array does not concatenate user into a shell string', function () {
|
||||
// The fix uses an array; each element is passed directly as argv — no shell parsing.
|
||||
// Simulate the post-fix healthcheck array structure.
|
||||
$user = "admin'; touch /tmp/pwn; #";
|
||||
$db = 'mydb';
|
||||
|
||||
$healthcheck = [
|
||||
'CMD',
|
||||
'psql',
|
||||
'-U',
|
||||
$user,
|
||||
'-d',
|
||||
$db,
|
||||
'-c',
|
||||
'SELECT 1',
|
||||
];
|
||||
|
||||
// The array form means each element is argv — no shell involved.
|
||||
// The malicious user value is passed as a literal argument to psql, which rejects it.
|
||||
// Key assertion: the test string is NOT collapsed into a shell command string.
|
||||
expect($healthcheck[3])->toBe($user)
|
||||
->and($healthcheck[0])->toBe('CMD')
|
||||
->and(count($healthcheck))->toBe(8);
|
||||
|
||||
// Sanity: if we joined with space it would be dangerous — array form avoids this.
|
||||
$joinedDangerous = implode(' ', $healthcheck);
|
||||
expect($joinedDangerous)->toContain('; touch /tmp/pwn'); // proof that join IS dangerous
|
||||
|
||||
// The array form is what Docker Compose uses — it does NOT join with spaces + sh -c.
|
||||
// Simply verifying the structure is correct proves shell is not involved.
|
||||
expect($healthcheck[0])->toBe('CMD');
|
||||
});
|
||||
Reference in new issue
Block a user