fix(database): tighten Postgres init script filename handling (#9681)
This commit is contained in:
commit
1cf6c7d0ae
5 changed files with 297 additions and 10 deletions
|
|
@ -301,9 +301,18 @@ private function generate_init_scripts()
|
|||
foreach ($this->database->init_scripts as $init_script) {
|
||||
$filename = data_get($init_script, 'filename');
|
||||
$content = data_get($init_script, 'content');
|
||||
|
||||
// Normalise filename without rejecting legacy values so previously created
|
||||
// init scripts keep deploying. basename() strips any directory components
|
||||
// (path traversal) and escapeshellarg() contains every shell metacharacter
|
||||
// in the tee target. Livewire / API validate new filenames up front.
|
||||
$filename = basename((string) $filename);
|
||||
|
||||
$target_path = "$this->configuration_dir/docker-entrypoint-initdb.d/{$filename}";
|
||||
$escaped_target = escapeshellarg($target_path);
|
||||
$content_base64 = base64_encode($content);
|
||||
$this->commands[] = "echo '{$content_base64}' | base64 -d | tee $this->configuration_dir/docker-entrypoint-initdb.d/{$filename} > /dev/null";
|
||||
$this->init_scripts[] = "$this->configuration_dir/docker-entrypoint-initdb.d/{$filename}";
|
||||
$this->commands[] = "echo '{$content_base64}' | base64 -d | tee {$escaped_target} > /dev/null";
|
||||
$this->init_scripts[] = $target_path;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -358,9 +358,14 @@ public function save_init_script($script)
|
|||
|
||||
if ($oldScript && $oldScript['filename'] !== $script['filename']) {
|
||||
try {
|
||||
// Validate and escape filename to prevent command injection
|
||||
validateShellSafePath($oldScript['filename'], 'init script filename');
|
||||
$old_file_path = "$configuration_dir/docker-entrypoint-initdb.d/{$oldScript['filename']}";
|
||||
// New filename is user-supplied — must be safe before accepting the rename.
|
||||
validateFilenameSafe($script['filename'], 'init script filename');
|
||||
|
||||
// Old filename may be a legacy value written before this validation existed.
|
||||
// basename() scopes the rm to the initdb.d directory; escapeshellarg() contains
|
||||
// any remaining shell-metachars. No validator — don't block cleanup of legacy rows.
|
||||
$old_filename = basename($oldScript['filename']);
|
||||
$old_file_path = "$configuration_dir/docker-entrypoint-initdb.d/{$old_filename}";
|
||||
$escapedOldPath = escapeshellarg($old_file_path);
|
||||
$delete_command = "rm -f {$escapedOldPath}";
|
||||
instant_remote_process([$delete_command], $this->server);
|
||||
|
|
@ -404,9 +409,11 @@ public function delete_init_script($script)
|
|||
$configuration_dir = database_configuration_dir().'/'.$container_name;
|
||||
|
||||
try {
|
||||
// Validate and escape filename to prevent command injection
|
||||
validateShellSafePath($script['filename'], 'init script filename');
|
||||
$file_path = "$configuration_dir/docker-entrypoint-initdb.d/{$script['filename']}";
|
||||
// Allow deletion of legacy rows with unsafe filenames so operators can clean up.
|
||||
// basename() scopes the rm to the initdb.d directory; escapeshellarg() keeps the
|
||||
// shell invocation safe regardless of the stored value.
|
||||
$safe_filename = basename($script['filename']);
|
||||
$file_path = "$configuration_dir/docker-entrypoint-initdb.d/{$safe_filename}";
|
||||
$escapedPath = escapeshellarg($file_path);
|
||||
|
||||
$command = "rm -f {$escapedPath}";
|
||||
|
|
@ -443,8 +450,8 @@ public function save_new_init_script()
|
|||
]);
|
||||
|
||||
try {
|
||||
// Validate filename to prevent command injection
|
||||
validateShellSafePath($this->new_filename, 'init script filename');
|
||||
// Validate filename to prevent path traversal and command injection
|
||||
validateFilenameSafe($this->new_filename, 'init script filename');
|
||||
} catch (Exception $e) {
|
||||
$this->dispatch('error', $e->getMessage());
|
||||
|
||||
|
|
|
|||
|
|
@ -157,6 +157,73 @@ function validateShellSafePath(string $input, string $context = 'path'): string
|
|||
return $input;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate that a filename is safe for use as a plain file name (no path components).
|
||||
*
|
||||
* Prevents path traversal attacks by rejecting directory separators, traversal
|
||||
* sequences, and null bytes, in addition to all shell metacharacters blocked by
|
||||
* validateShellSafePath(). Intended for user-supplied filenames such as PostgreSQL
|
||||
* init script names that are later written to a specific directory on the host.
|
||||
*
|
||||
* @param string $input The filename to validate
|
||||
* @param string $context Descriptive name for error messages (e.g., 'init script filename')
|
||||
* @return string The validated input (unchanged if valid)
|
||||
*
|
||||
* @throws Exception If dangerous characters or path traversal sequences are detected
|
||||
*/
|
||||
function validateFilenameSafe(string $input, string $context = 'filename'): string
|
||||
{
|
||||
// First apply shell-metachar checks
|
||||
validateShellSafePath($input, $context);
|
||||
|
||||
// Reject NUL bytes (can be used to truncate path strings in some contexts)
|
||||
if (str_contains($input, "\0")) {
|
||||
throw new Exception(
|
||||
"Invalid {$context}: contains null byte. ".
|
||||
'Null bytes are not allowed in filenames for security reasons.'
|
||||
);
|
||||
}
|
||||
|
||||
// Reject directory separators — filename must be a single path component
|
||||
if (str_contains($input, '/') || str_contains($input, '\\')) {
|
||||
throw new Exception(
|
||||
"Invalid {$context}: directory separators ('/' or '\\') are not allowed. ".
|
||||
'Provide a plain filename without path components.'
|
||||
);
|
||||
}
|
||||
|
||||
// Reject path traversal sequences (catches encoded or unusual forms)
|
||||
if (str_contains($input, '..')) {
|
||||
throw new Exception(
|
||||
"Invalid {$context}: path traversal sequence ('..') is not allowed."
|
||||
);
|
||||
}
|
||||
|
||||
// Reject shell globbing / expansion metacharacters and whitespace that would
|
||||
// split the filename into additional shell arguments if ever interpolated
|
||||
// unquoted (defence in depth on top of escapeshellarg() at call sites).
|
||||
$shellExpansionChars = [
|
||||
' ' => 'whitespace',
|
||||
'*' => 'glob wildcard',
|
||||
'?' => 'glob wildcard',
|
||||
'[' => 'glob character class',
|
||||
']' => 'glob character class',
|
||||
'~' => 'tilde expansion',
|
||||
'"' => 'double quote',
|
||||
"'" => 'single quote',
|
||||
];
|
||||
|
||||
foreach ($shellExpansionChars as $char => $description) {
|
||||
if (str_contains($input, $char)) {
|
||||
throw new Exception(
|
||||
"Invalid {$context}: contains forbidden character '{$char}' ({$description})."
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
return $input;
|
||||
}
|
||||
|
||||
/**
|
||||
* Validate that a databases_to_backup input string is safe from command injection.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -74,3 +74,69 @@
|
|||
expect(fn () => validateShellSafePath('setup_db.sql', 'init script filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
// Path traversal — GHSA-mv4c-9x67-rrmv regression tests
|
||||
test('postgresql init script rejects path traversal with ../ sequence', function () {
|
||||
expect(fn () => validateFilenameSafe('../../../etc/cron.d/pwn', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('postgresql init script rejects path traversal targeting /etc/cron.d', function () {
|
||||
expect(fn () => validateFilenameSafe('../../../../../etc/cron.d/k4zrce', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('postgresql init script rejects absolute path', function () {
|
||||
expect(fn () => validateFilenameSafe('/etc/passwd', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('postgresql init script rejects filename with forward slash', function () {
|
||||
expect(fn () => validateFilenameSafe('subdir/evil.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('postgresql init script rejects filename with backslash', function () {
|
||||
expect(fn () => validateFilenameSafe('subdir\\evil.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('postgresql init script rejects double-dot without slashes', function () {
|
||||
expect(fn () => validateFilenameSafe('..', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('postgresql init script rejects null byte injection', function () {
|
||||
expect(fn () => validateFilenameSafe("init.sql\0../../etc/passwd", 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('postgresql init script accepts legitimate filenames via validateFilenameSafe', function () {
|
||||
expect(fn () => validateFilenameSafe('init.sql', 'init script filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateFilenameSafe('01_schema.sql', 'init script filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateFilenameSafe('init-script.sh', 'init script filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
// Write-site defence — basename() + escapeshellarg() keep legacy/bad rows safe
|
||||
test('basename() strips path traversal from legacy filenames at write site', function () {
|
||||
expect(basename('../../../etc/cron.d/pwn'))->toBe('pwn');
|
||||
expect(basename('/etc/passwd'))->toBe('passwd');
|
||||
expect(basename('subdir/evil.sql'))->toBe('evil.sql');
|
||||
});
|
||||
|
||||
test('escapeshellarg() neutralises shell metacharacters in tee target', function () {
|
||||
// Simulates how StartPostgresql::generate_init_scripts() builds the tee argument
|
||||
$configuration_dir = '/data/coolify/databases/abc123';
|
||||
$legacy_filename = basename('foo bar*.sql;rm -rf /');
|
||||
$target = "$configuration_dir/docker-entrypoint-initdb.d/{$legacy_filename}";
|
||||
$escaped = escapeshellarg($target);
|
||||
|
||||
// Single-quoted in POSIX sh means no expansion / no extra args regardless of contents.
|
||||
expect($escaped)->toStartWith("'")->toEndWith("'");
|
||||
expect($escaped)->toContain('foo bar*.sql;rm -rf');
|
||||
});
|
||||
|
|
|
|||
138
tests/Unit/ValidateFilenameSafeTest.php
Normal file
138
tests/Unit/ValidateFilenameSafeTest.php
Normal file
|
|
@ -0,0 +1,138 @@
|
|||
<?php
|
||||
|
||||
test('allows plain filenames without special characters', function () {
|
||||
$validNames = [
|
||||
'init.sql',
|
||||
'01_schema.sql',
|
||||
'setup-db.sql',
|
||||
'create_test_db.sql',
|
||||
'init-script.sh',
|
||||
'UPPERCASE.SQL',
|
||||
'mixed_Case-File.sql',
|
||||
'file123.sql',
|
||||
'a',
|
||||
];
|
||||
|
||||
foreach ($validNames as $name) {
|
||||
expect(fn () => validateFilenameSafe($name, 'init script filename'))
|
||||
->not->toThrow(Exception::class, "Expected '{$name}' to pass");
|
||||
}
|
||||
});
|
||||
|
||||
test('rejects path traversal with ../', function () {
|
||||
expect(fn () => validateFilenameSafe('../../../etc/cron.d/pwn', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects path traversal with .. alone', function () {
|
||||
expect(fn () => validateFilenameSafe('..', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects path traversal embedded in filename', function () {
|
||||
expect(fn () => validateFilenameSafe('foo..bar', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects forward slash directory separator', function () {
|
||||
expect(fn () => validateFilenameSafe('foo/bar.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects backslash directory separator', function () {
|
||||
expect(fn () => validateFilenameSafe('foo\\bar.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects absolute path starting with slash', function () {
|
||||
expect(fn () => validateFilenameSafe('/etc/passwd', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects absolute Windows-style path', function () {
|
||||
expect(fn () => validateFilenameSafe('C:\\Windows\\System32\\cmd.exe', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects null byte injection', function () {
|
||||
expect(fn () => validateFilenameSafe("init.sql\0../../etc/passwd", 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects shell command substitution (inherits from validateShellSafePath)', function () {
|
||||
expect(fn () => validateFilenameSafe('$(whoami).sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects backtick command substitution', function () {
|
||||
expect(fn () => validateFilenameSafe('`id`.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects semicolon command separator', function () {
|
||||
expect(fn () => validateFilenameSafe('init.sql;rm -rf /', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects pipe operator', function () {
|
||||
expect(fn () => validateFilenameSafe('init.sql|whoami', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects redirect operators', function () {
|
||||
expect(fn () => validateFilenameSafe('init.sql>/etc/passwd', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects mixed traversal and shell injection', function () {
|
||||
expect(fn () => validateFilenameSafe('../etc/cron.d/$(id)', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('error message contains context string', function () {
|
||||
try {
|
||||
validateFilenameSafe('../evil', 'init script filename');
|
||||
expect(false)->toBeTrue('Should have thrown');
|
||||
} catch (Exception $e) {
|
||||
expect($e->getMessage())->toContain('init script filename');
|
||||
}
|
||||
});
|
||||
|
||||
test('handles empty string without throwing', function () {
|
||||
expect(fn () => validateFilenameSafe('', 'init script filename'))
|
||||
->not->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects whitespace inside filename (would split into extra tee arg)', function () {
|
||||
expect(fn () => validateFilenameSafe('foo bar.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects glob wildcards', function () {
|
||||
expect(fn () => validateFilenameSafe('init*.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateFilenameSafe('init?.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects glob character class brackets', function () {
|
||||
expect(fn () => validateFilenameSafe('init[abc].sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects tilde expansion', function () {
|
||||
expect(fn () => validateFilenameSafe('~/evil.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateFilenameSafe('~root', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
|
||||
test('rejects single and double quotes', function () {
|
||||
expect(fn () => validateFilenameSafe("foo'bar.sql", 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
|
||||
expect(fn () => validateFilenameSafe('foo"bar.sql', 'init script filename'))
|
||||
->toThrow(Exception::class);
|
||||
});
|
||||
Loading…
Reference in a new issue