Security: fix 8 code-review findings

- install.sh: replace /usr/sbin/ufw * with scoped subcommands
- install.sh: remove /usr/bin/curl * and /usr/bin/env * NOPASSWD (trivial root escalation)
- PHPManager: switchVersion() uses sudo rm -f instead of unlink() for old pool
- PHPManager: updateConfig() SQLite syntax (ON CONFLICT / datetime('now'))
- WordPressManager: cloneStaging() escapeshellarg() on all shell-interpolated paths
- WordPressManager: delete() removes DB record before filesystem to avoid phantom records
- WordPressManager: ensureWpCli() validates download size and enforces 30s timeout

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-06-10 12:32:02 +00:00
parent 5037633f5f
commit 2fa1f10901
3 changed files with 29 additions and 19 deletions
+10 -9
View File
@@ -637,7 +637,15 @@ log "Fail2Ban configured"
cat > /etc/sudoers.d/novacpx-firewall <<SUDOERS cat > /etc/sudoers.d/novacpx-firewall <<SUDOERS
Defaults:www-data !requiretty Defaults:www-data !requiretty
# Firewall / security # Firewall / security
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw * www-data ALL=(root) NOPASSWD: /usr/sbin/ufw status
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw status verbose
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw allow *
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw deny *
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw delete *
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw reload
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw enable
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw disable
www-data ALL=(root) NOPASSWD: /usr/sbin/ufw logging *
www-data ALL=(root) NOPASSWD: /usr/bin/fail2ban-client * www-data ALL=(root) NOPASSWD: /usr/bin/fail2ban-client *
# Web servers # Web servers
www-data ALL=(root) NOPASSWD: /bin/systemctl start apache2 www-data ALL=(root) NOPASSWD: /bin/systemctl start apache2
@@ -711,16 +719,9 @@ www-data ALL=(root) NOPASSWD: /bin/systemctl reload php*-fpm
www-data ALL=(root) NOPASSWD: /bin/systemctl restart php*-fpm www-data ALL=(root) NOPASSWD: /bin/systemctl restart php*-fpm
www-data ALL=(root) NOPASSWD: /bin/systemctl start php*-fpm www-data ALL=(root) NOPASSWD: /bin/systemctl start php*-fpm
www-data ALL=(root) NOPASSWD: /bin/systemctl stop php*-fpm www-data ALL=(root) NOPASSWD: /bin/systemctl stop php*-fpm
# DB tool installation privileges # Web config file management (scoped paths only)
www-data ALL=(root) NOPASSWD: /usr/bin/gpg *
www-data ALL=(root) NOPASSWD: /usr/bin/curl *
www-data ALL=(root) NOPASSWD: /usr/sbin/debconf-set-selections *
www-data ALL=(root) NOPASSWD: /usr/bin/tee /etc/apt/sources.list.d/*
www-data ALL=(root) NOPASSWD: /usr/bin/tee /usr/share/keyrings/*
www-data ALL=(root) NOPASSWD: /usr/bin/tee /etc/nginx/conf.d/* www-data ALL=(root) NOPASSWD: /usr/bin/tee /etc/nginx/conf.d/*
www-data ALL=(root) NOPASSWD: /usr/bin/tee /etc/apache2/conf-enabled/* www-data ALL=(root) NOPASSWD: /usr/bin/tee /etc/apache2/conf-enabled/*
www-data ALL=(root) NOPASSWD: /usr/pgadmin4/bin/setup-web.sh *
www-data ALL=(root) NOPASSWD: /usr/bin/env *
SUDOERS SUDOERS
chmod 440 /etc/sudoers.d/novacpx-firewall chmod 440 /etc/sudoers.d/novacpx-firewall
log "Sudoers rules installed" log "Sudoers rules installed"
+8 -5
View File
@@ -65,7 +65,8 @@ php_value[max_execution_time] = 30
// Remove old pool, create new one // Remove old pool, create new one
$oldPool = str_replace('{ver}', $oldVer, self::$poolDir) . "/{$acct['username']}.conf"; $oldPool = str_replace('{ver}', $oldVer, self::$poolDir) . "/{$acct['username']}.conf";
if (file_exists($oldPool)) { unlink($oldPool); self::reloadFPM($oldVer); } shell_exec("sudo rm -f " . escapeshellarg($oldPool) . " 2>/dev/null");
self::reloadFPM($oldVer);
self::createPool($acct['username'], $newVer); self::createPool($acct['username'], $newVer);
@@ -100,10 +101,12 @@ php_value[max_execution_time] = 30
self::reloadFPM($acct['php_version']); self::reloadFPM($acct['php_version']);
$db->execute( $db->execute(
"INSERT INTO php_configs (account_id, php_version, memory_limit, max_execution_time, upload_max_filesize, post_max_size) "INSERT INTO php_configs (account_id, php_version, memory_limit, max_execution_time, upload_max_filesize, post_max_size, updated_at)
VALUES (?,?,?,?,?,?) VALUES (?,?,?,?,?,?,datetime('now'))
ON DUPLICATE KEY UPDATE memory_limit=VALUES(memory_limit), max_execution_time=VALUES(max_execution_time), ON CONFLICT(account_id) DO UPDATE SET php_version=excluded.php_version,
upload_max_filesize=VALUES(upload_max_filesize), post_max_size=VALUES(post_max_size), updated_at=NOW()", memory_limit=excluded.memory_limit, max_execution_time=excluded.max_execution_time,
upload_max_filesize=excluded.upload_max_filesize, post_max_size=excluded.post_max_size,
updated_at=excluded.updated_at",
[$accountId, $acct['php_version'], $cfg['memory_limit'] ?? '256M', $cfg['max_execution_time'] ?? 30, [$accountId, $acct['php_version'], $cfg['memory_limit'] ?? '256M', $cfg['max_execution_time'] ?? 30,
$cfg['upload_max_filesize'] ?? '64M', $cfg['post_max_size'] ?? '64M'] $cfg['upload_max_filesize'] ?? '64M', $cfg['post_max_size'] ?? '64M']
); );
+11 -5
View File
@@ -111,7 +111,7 @@ class WordPressManager {
$stagingRoot = dirname($docRoot) . rtrim($stagingPath, '/'); $stagingRoot = dirname($docRoot) . rtrim($stagingPath, '/');
// Copy files // Copy files
$this->exec("cp -r {$docRoot} {$stagingRoot}"); $this->exec("cp -r " . escapeshellarg($docRoot) . " " . escapeshellarg($stagingRoot));
// Clone DB // Clone DB
$stagingDb = $install['db_name'] . '_staging'; $stagingDb = $install['db_name'] . '_staging';
@@ -119,7 +119,7 @@ class WordPressManager {
$this->getProvDb()->exec("CREATE DATABASE IF NOT EXISTS `{$stagingDb}`"); $this->getProvDb()->exec("CREATE DATABASE IF NOT EXISTS `{$stagingDb}`");
$this->getProvDb()->exec("CREATE USER IF NOT EXISTS '{$stagingDb}'@'localhost' IDENTIFIED BY '{$stagingDbPw}'"); $this->getProvDb()->exec("CREATE USER IF NOT EXISTS '{$stagingDb}'@'localhost' IDENTIFIED BY '{$stagingDbPw}'");
$this->getProvDb()->exec("GRANT ALL ON `{$stagingDb}`.* TO '{$stagingDb}'@'localhost'"); $this->getProvDb()->exec("GRANT ALL ON `{$stagingDb}`.* TO '{$stagingDb}'@'localhost'");
$this->exec("mysqldump {$install['db_name']} | mysql {$stagingDb}"); $this->exec("mysqldump " . escapeshellarg($install['db_name']) . " | mysql " . escapeshellarg($stagingDb));
// Update staging wp-config // Update staging wp-config
$this->wp($stagingRoot, "config set DB_NAME {$stagingDb}", $sysUser); $this->wp($stagingRoot, "config set DB_NAME {$stagingDb}", $sysUser);
@@ -141,10 +141,11 @@ class WordPressManager {
// ── Delete ──────────────────────────────────────────────────────────────── // ── Delete ────────────────────────────────────────────────────────────────
public function delete(int $id): bool { public function delete(int $id): bool {
[$install, $sysUser, $docRoot] = $this->resolve($id); [$install, $sysUser, $docRoot] = $this->resolve($id);
$this->exec("rm -rf {$docRoot}"); // Remove DB record first so a failed filesystem cleanup doesn't leave a phantom record
$this->db->prepare("DELETE FROM wordpress_installs WHERE id=?")->execute([$id]);
$this->exec("rm -rf " . escapeshellarg($docRoot));
$this->getProvDb()->exec("DROP DATABASE IF EXISTS `{$install['db_name']}`"); $this->getProvDb()->exec("DROP DATABASE IF EXISTS `{$install['db_name']}`");
$this->getProvDb()->exec("DROP USER IF EXISTS '{$install['db_user']}'@'localhost'"); $this->getProvDb()->exec("DROP USER IF EXISTS '{$install['db_user']}'@'localhost'");
$this->db->prepare("DELETE FROM wordpress_installs WHERE id=?")->execute([$id]);
return true; return true;
} }
@@ -245,7 +246,12 @@ class WordPressManager {
private function ensureWpCli(): void { private function ensureWpCli(): void {
if (!file_exists($this->wpcli)) { if (!file_exists($this->wpcli)) {
file_put_contents('/tmp/wp-cli.phar', file_get_contents('https://raw.githubusercontent.com/wp-cli/builds/gh-pages/phar/wp-cli.phar')); $ctx = stream_context_create(['http' => ['timeout' => 30]]);
$data = @file_get_contents('https://raw.githubusercontent.com/wp-cli/builds/gh-pages/phar/wp-cli.phar', false, $ctx);
if (!$data || strlen($data) < 100000) {
throw new \RuntimeException("Failed to download WP-CLI (received " . strlen((string)$data) . " bytes)");
}
file_put_contents('/tmp/wp-cli.phar', $data);
rename('/tmp/wp-cli.phar', $this->wpcli); rename('/tmp/wp-cli.phar', $this->wpcli);
chmod($this->wpcli, 0755); chmod($this->wpcli, 0755);
} }