Fix arbitrary file read via restore upload path (security)
stageRestoreUploadFromDirectAdminTemp() accepted the raw source_file_upload POST value as a literal filesystem path with only an is_file() check - no ownership or allowed-root validation, unlike the sibling queueRestoreFromFilePath(). isAllowedSqlFile() only checks the attacker-controlled display filename, never the real source path. Since the whole plugin runs as root, any authenticated DirectAdmin user could stage and restore-import any file readable by root on the server (e.g. other users' alt-mysql.conf credentials), leaking its contents via MySQL import error output or the resulting database. Remove the unsafe raw-path candidate entirely; only accept files whose basename matches one inside the already-safe, fixed set of known DirectAdmin/system temp directories.
This commit is contained in:
@@ -437,10 +437,12 @@ final class BackupQueueService
|
||||
throw new RuntimeException('Nie można odczytać przesłanego pliku backupu.');
|
||||
}
|
||||
|
||||
// Security: never trust $uploadReference as a literal path - it comes
|
||||
// straight from user-controlled POST data, and this whole plugin runs
|
||||
// as root. Only accept a file whose *basename* matches one of the
|
||||
// known DirectAdmin/system temp directories below, never an
|
||||
// arbitrary absolute path chosen by the caller.
|
||||
$sourceCandidates = [];
|
||||
if (str_starts_with($uploadReference, '/') && is_file($uploadReference)) {
|
||||
$sourceCandidates[] = $uploadReference;
|
||||
}
|
||||
$tmpDirs = [
|
||||
(string)(getenv('UPLOAD_TMP_DIR') ?: ''),
|
||||
(string)(getenv('DA_UPLOAD_TMP_DIR') ?: ''),
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
<?php
|
||||
declare(strict_types=1);
|
||||
|
||||
$pluginTempRoot = sys_get_temp_dir() . '/altmysql-plugin-root-' . bin2hex(random_bytes(4));
|
||||
mkdir($pluginTempRoot, 0700, true);
|
||||
define('PLUGIN_ROOT', $pluginTempRoot);
|
||||
|
||||
$pluginDir = dirname(__DIR__);
|
||||
require_once $pluginDir . '/exec/lib/Settings.php';
|
||||
require_once $pluginDir . '/exec/lib/DirectAdminUser.php';
|
||||
require_once $pluginDir . '/exec/lib/BackupQueueService.php';
|
||||
|
||||
function fail(string $message): void
|
||||
{
|
||||
fwrite(STDERR, "FAIL: {$message}\n");
|
||||
exit(1);
|
||||
}
|
||||
|
||||
function assert_true(bool $condition, string $message): void
|
||||
{
|
||||
if (!$condition) {
|
||||
fail($message);
|
||||
}
|
||||
}
|
||||
|
||||
function load_settings(string $contents): Settings
|
||||
{
|
||||
$path = tempnam(sys_get_temp_dir(), 'altmysql-settings-');
|
||||
if ($path === false) {
|
||||
fail('cannot create temp settings file');
|
||||
}
|
||||
file_put_contents($path, $contents);
|
||||
$settings = Settings::load($path);
|
||||
unlink($path);
|
||||
return $settings;
|
||||
}
|
||||
|
||||
function make_da_user(Settings $settings, string $username): DirectAdminUser
|
||||
{
|
||||
$class = new ReflectionClass(DirectAdminUser::class);
|
||||
/** @var DirectAdminUser $user */
|
||||
$user = $class->newInstanceWithoutConstructor();
|
||||
foreach ([
|
||||
'username' => $username,
|
||||
'prefix' => $username . '_',
|
||||
'userConf' => [],
|
||||
'packageConf' => [],
|
||||
'settings' => $settings,
|
||||
] as $property => $value) {
|
||||
$prop = $class->getProperty($property);
|
||||
$prop->setAccessible(true);
|
||||
$prop->setValue($user, $value);
|
||||
}
|
||||
return $user;
|
||||
}
|
||||
|
||||
$settings = load_settings('');
|
||||
$daUser = make_da_user($settings, 'demo');
|
||||
$queue = new BackupQueueService($settings, $daUser);
|
||||
|
||||
// A path outside every recognized DirectAdmin/system temp directory must be
|
||||
// rejected, even if it exists and is readable - it must not be treated as a
|
||||
// legitimate upload reference just because the caller supplied it verbatim.
|
||||
$secretDir = sys_get_temp_dir() . '/altmysql-secret-' . bin2hex(random_bytes(4));
|
||||
mkdir($secretDir, 0700, true);
|
||||
$secretFile = $secretDir . '/secret_credentials.sql';
|
||||
file_put_contents($secretFile, 'SECRET CONTENT THAT MUST NEVER BE COPIED');
|
||||
|
||||
$threw = false;
|
||||
try {
|
||||
$queue->stageRestoreUploadFromDirectAdminTemp($secretFile, 'harmless.sql');
|
||||
} catch (Throwable $e) {
|
||||
$threw = true;
|
||||
}
|
||||
assert_true($threw, 'a path outside the known DirectAdmin/system temp directories must be rejected, not staged');
|
||||
|
||||
// A legitimate reference - a file that DirectAdmin actually placed directly
|
||||
// inside a recognized temp directory - must still work.
|
||||
$legitFile = sys_get_temp_dir() . '/altmysql-da-upload-' . bin2hex(random_bytes(4)) . '.sql';
|
||||
file_put_contents($legitFile, 'SELECT 1;');
|
||||
|
||||
$stagedPath = $queue->stageRestoreUploadFromDirectAdminTemp($legitFile, 'legit.sql');
|
||||
assert_true(is_file($stagedPath), 'a file directly inside a recognized temp directory must still be staged');
|
||||
assert_true(
|
||||
trim((string)file_get_contents($stagedPath)) === 'SELECT 1;',
|
||||
'the staged file must contain the legitimate source content'
|
||||
);
|
||||
|
||||
// cleanup
|
||||
@unlink($secretFile);
|
||||
@rmdir($secretDir);
|
||||
@unlink($legitFile);
|
||||
@unlink($stagedPath);
|
||||
exec('rm -rf ' . escapeshellarg($pluginTempRoot));
|
||||
|
||||
echo "backup_restore_upload_path_test: OK\n";
|
||||
Reference in New Issue
Block a user