Merge pull request 'security: fix auth bypass, XSS, IDOR, command injection, session hardening, rate limiting' (#86) from fix/policy-container-width into master

Reviewed-on: #86
Reviewed-by: rafaga21 <rafaelminaya20@hotmail.com>

database/migrations/015_rate_limits.sql

@@ -0,0 +1,8 @@
+-- Migration 015: Rate limiting storage
+CREATE TABLE IF NOT EXISTS rate_limits (
+    id INT AUTO_INCREMENT PRIMARY KEY,
+    identifier VARCHAR(255) NOT NULL,
+    attempts INT NOT NULL DEFAULT 1,
+    first_attempt_at BIGINT NOT NULL,
+    UNIQUE KEY idx_identifier (identifier)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4;

routes/web.php

@@ -31,7 +31,7 @@ $router->get('/login', [AuthController::class, 'loginForm']);
 $router->post('/login', [AuthController::class, 'login'], [CSRFMiddleware::class, RateLimitMiddleware::class]);
 $router->get('/register', [AuthController::class, 'registerForm']);
 $router->post('/register', [AuthController::class, 'register'], [CSRFMiddleware::class]);
-$router->get('/logout', [AuthController::class, 'logout']);
+$router->post('/logout', [AuthController::class, 'logout'], [CSRFMiddleware::class]);
 
 $router->get('/profile', [AuthController::class, 'profile'], [AuthMiddleware::class]);
 $router->post('/profile', [AuthController::class, 'updateProfile'], [AuthMiddleware::class, CSRFMiddleware::class]);
@@ -115,8 +115,8 @@ $router->put('/api/servers/:id', [ApiController::class, 'serverUpdate']);
 $router->delete('/api/servers/:id', [ApiController::class, 'serverDelete']);
 $router->get('/api/servers/:id/metrics', [ApiController::class, 'serverMetrics']);
 $router->post('/api/servers/:id/execute', [ApiController::class, 'executeCommand']);
-$router->post('/api/auth/login', [ApiController::class, 'login']);
+$router->post('/api/auth/login', [ApiController::class, 'login'], [RateLimitMiddleware::class]);
-$router->post('/api/auth/register', [ApiController::class, 'register']);
+$router->post('/api/auth/register', [ApiController::class, 'register'], [RateLimitMiddleware::class]);
 $router->get('/api/app-version', [ApiController::class, 'appVersion']);
 $router->get('/api/servers/:id/services', [ApiController::class, 'serverServices']);
 $router->post('/api/servers/:id/reboot', [ApiController::class, 'serverReboot']);

src/Controllers/ApiController.php

@@ -341,6 +341,25 @@ class ApiController
             $this->view->json(['error' => 'Command is required'], 400);
         }
 
+        $blockedPatterns = [
+            '/\brm\s+\-rf\s+\//',
+            '/\bmkfs\b/',
+            '/\bdd\s+if\=/',
+            '/\:\s*\{\s*:\s*\|[^\}]*\}\s*;/',
+            '/\bshutdown\b/',
+            '/\breboot\b/',
+            '/\bhalt\b/',
+            '/\bpoweroff\b/',
+        ];
+        foreach ($blockedPatterns as $pattern) {
+            if (preg_match($pattern, $command)) {
+                $this->view->json([
+                    'success' => false,
+                    'error' => 'This command has been blocked for safety reasons.',
+                ], 400);
+            }
+        }
+
         $serverModel = new Server();
         $server = $serverModel->findById($id);
 
@@ -378,10 +397,9 @@ class ApiController
             ]);
         } catch (\Throwable $e) {
             $this->view->json([
-                'error' => 'Command execution failed: ' . $e->getMessage(),
+                'error' => 'Command execution failed.',
             ], 500);
         }
-    }
 
     public function users(): void
     {
@@ -496,6 +514,12 @@ class ApiController
 
     public function register(): void
     {
+        $user = $this->authenticateRequest();
+
+        if (!in_array($user['role'], ['super_admin', 'admin'], true)) {
+            $this->view->json(['error' => 'Forbidden'], 403);
+        }
+
         $input = json_decode(file_get_contents('php://input'), true) ?? $_POST;
 
         $validator = new Validator();
@@ -504,6 +528,7 @@ class ApiController
             'email' => 'required|email',
             'password' => 'required|min:8',
             'confirm_password' => 'required',
+            'role' => 'in:operator,admin',
         ];
 
         if (!$validator->validate($input, $rules)) {
@@ -522,6 +547,13 @@ class ApiController
             return;
         }
 
+        $role = in_array($input['role'] ?? 'operator', ['operator', 'admin'], true)
+            ? $input['role'] : 'operator';
+
+        if ($role === 'admin' && $user['role'] !== 'super_admin') {
+            $this->view->json(['error' => 'Only super admins can create admin users'], 403);
+        }
+
         $userModel = new User();
 
         $existing = $userModel->findByUsername($input['username']);
@@ -546,7 +578,7 @@ class ApiController
             'username' => $input['username'],
             'email' => $input['email'],
             'password' => $input['password'],
-            'role' => 'admin',
+            'role' => $role,
             'is_active' => 1,
         ]);
 

src/Controllers/ServerController.php

@@ -1261,8 +1261,8 @@ class ServerController
                     $this->view->json(['success' => false, 'message' => 'Domain is required']);
                 }
 
-                $domainList = preg_replace('/\s+/', ',', $domains);
+                $domainList = escapeshellarg(preg_replace('/\s+/', ',', $domains));
-                $emailArg = !empty($email) ? "--email {$email}" : '--register-unsafely-without-email';
+                $emailArg = !empty($email) ? '--email ' . escapeshellarg($email) : '--register-unsafely-without-email';
                 $cmd = "sudo certbot --nginx -d {$domainList} {$emailArg} --non-interactive --agree-tos 2>&1; echo EXIT:\$?";
                 $result = $ssh->exec($cmd);
                 $exitCode = (int) trim(explode('EXIT:', $result['output'] ?? 'EXIT:-1')[1] ?? '-1');
@@ -1773,20 +1773,25 @@ class ServerController
             $this->view->json(['success' => false, 'message' => 'Invalid action or service']);
         }
 
+        $sanitizedService = preg_replace('/[^a-zA-Z0-9\-_@.]/', '', $service);
+        if ($sanitizedService === '') {
+            $this->view->json(['success' => false, 'message' => 'Invalid service name']);
+        }
+
         try {
             $ssh = new SSHService();
             if (!$ssh->connect($server)) {
                 throw new \RuntimeException('Could not establish SSH connection');
             }
-            $this->auditService->logServerAction($id, "service_{$action}", "{$service} {$action}");
+            $this->auditService->logServerAction($id, "service_{$action}", "{$sanitizedService} {$action}");
 
-            $result = $ssh->exec("sudo systemctl {$action} {$service} 2>&1; echo EXIT:\$?");
+            $result = $ssh->exec("sudo systemctl {$action} {$sanitizedService} 2>&1; echo EXIT:\$?");
             $exitCode = (int) trim(explode('EXIT:', $result['output'] ?? 'EXIT:-1')[1] ?? '-1');
             $ssh->disconnect();
 
             $this->view->json([
                     'success' => $exitCode === 0,
-                'message' => $exitCode === 0 ? "Service '{$service}' {$action}ed." : "Failed to {$action} '{$service}'.",
+                    'message' => $exitCode === 0 ? "Service '{$sanitizedService}' {$action}ed." : "Failed to {$action} '{$sanitizedService}'.",
                     'output' => trim($result['output']),
                 ]);
         } catch (\Throwable $e) {

src/Controllers/TerminalController.php

@@ -66,14 +66,22 @@ class TerminalController
             $this->view->json(['success' => false, 'message' => 'Command is required']);
         }
 
-        $blockedCommands = ['rm -rf /', 'mkfs', 'dd if=', ':(){ :|:& };:'];
+        $blockedPatterns = [
-        foreach ($blockedCommands as $blocked) {
+            '/\brm\s+\-rf\s+\//',
-            if (stripos($command, $blocked) !== false) {
+            '/\bmkfs\b/',
+            '/\bdd\s+if\=/',
+            '/\:\s*\{\s*:\s*\|[^\}]*\}\s*;/',
+            '/\bshutdown\b/',
+            '/\breboot\b/',
+            '/\bhalt\b/',
+            '/\bpoweroff\b/',
+        ];
+        foreach ($blockedPatterns as $pattern) {
+            if (preg_match($pattern, $command)) {
                 $this->auditService->log('blocked_command', 'server', $id, [
                     'command' => $command,
                     'reason' => 'Blocked dangerous command pattern',
                 ]);
-
                 $this->view->json([
                     'success' => false,
                     'message' => 'This command has been blocked for safety reasons.',
@@ -111,13 +119,17 @@ class TerminalController
         } catch (\Throwable $e) {
             $this->view->json([
                 'success' => false,
-                'message' => 'Failed to execute command: ' . $e->getMessage(),
+                'message' => 'Failed to execute command.',
             ]);
         }
-    }
 
     public function history(int $id): void
     {
+        $userId = (int) Session::get('user_id');
+        if (!$this->serverModel->canView($id, $userId)) {
+            $this->view->json(['success' => false, 'message' => 'Forbidden'], 403);
+        }
+
         $commandHistory = new CommandHistory();
         $history = $commandHistory->getByServer($id, 100);
 
@@ -129,6 +141,11 @@ class TerminalController
 
     public function clearHistory(int $id): void
     {
+        $userId = (int) Session::get('user_id');
+        if (!$this->serverModel->canView($id, $userId)) {
+            $this->view->json(['success' => false, 'message' => 'Forbidden'], 403);
+        }
+
         $commandHistory = new CommandHistory();
         $commandHistory->clearHistory($id);
 

src/Middleware/RateLimitMiddleware.php

@@ -4,7 +4,6 @@ declare(strict_types=1);
 
 namespace ServerManager\Middleware;
 
-use ServerManager\Core\Session;
 use ServerManager\Core\Database;
 use ServerManager\Core\App;
 
@@ -17,24 +16,50 @@ class RateLimitMiddleware
         $decayMinutes = $config['decay_minutes'] ?? 15;
 
         $ip = $_SERVER['REMOTE_ADDR'] ?? '127.0.0.1';
-        $key = 'rate_limit_' . $ip . '_' . ($_SERVER['REQUEST_URI'] ?? '/');
+        $identifier = hash('sha256', $ip . '_' . ($_SERVER['REQUEST_URI'] ?? '/'));
+        $now = time();
+        $windowStart = $now - ($decayMinutes * 60);
 
-        $attempts = Session::get($key, ['count' => 0, 'first_attempt' => 0]);
+        $db = Database::getInstance();
 
-        if ($attempts['first_attempt'] && (time() - $attempts['first_attempt']) > ($decayMinutes * 60)) {
+        $existing = $db->fetch(
-            $attempts = ['count' => 0, 'first_attempt' => 0];
+            'SELECT attempts, first_attempt_at FROM rate_limits WHERE identifier = ?',
+            [$identifier]
+        );
+
+        if ($existing) {
+            if ((int) $existing['first_attempt_at'] < $windowStart) {
+                $db->query(
+                    'UPDATE rate_limits SET attempts = 1, first_attempt_at = ? WHERE identifier = ?',
+                    [$now, $identifier]
+                );
+                $attempts = 1;
+            } else {
+                $attempts = (int) $existing['attempts'] + 1;
+                $db->query(
+                    'UPDATE rate_limits SET attempts = ? WHERE identifier = ?',
+                    [$attempts, $identifier]
+                );
+            }
+        } else {
+            $db->insert('rate_limits', [
+                'identifier' => $identifier,
+                'attempts' => 1,
+                'first_attempt_at' => $now,
+            ]);
+            $attempts = 1;
         }
 
-        $attempts['count']++;
+        if (mt_rand(1, 100) <= 5) {
-
+            $db->query(
-        if ($attempts['first_attempt'] === 0) {
+                'DELETE FROM rate_limits WHERE first_attempt_at < ?',
-            $attempts['first_attempt'] = time();
+                [$windowStart]
+            );
         }
 
-        Session::set($key, $attempts);
+        if ($attempts > $maxAttempts) {
-
+            $firstAttemptAt = $existing ? (int) $existing['first_attempt_at'] : $now;
-        if ($attempts['count'] > $maxAttempts) {
+            $retryAfter = ($decayMinutes * 60) - ($now - $firstAttemptAt);
-            $retryAfter = ($decayMinutes * 60) - (time() - $attempts['first_attempt']);
 
             if ($this->isApiRequest()) {
                 http_response_code(429);

src/Models/Server.php

@@ -15,6 +15,16 @@ class Server
     private Encryption $encryption;
     private array $encryptedFields = ['ssh_password', 'ssh_key'];
 
+    private array $fillable = [
+        'name', 'ip_address', 'ssh_port', 'ssh_user', 'ssh_password', 'ssh_key',
+        'description', 'group_name', 'is_active', 'current_status', 'status',
+        'agent_key', 'agent_installed', 'agent_install_path', 'agent_last_seen_at',
+        'created_by', 'created_at', 'updated_at',
+        'threshold_cpu_warning', 'threshold_cpu_critical',
+        'threshold_ram_warning', 'threshold_ram_critical',
+        'threshold_disk_warning', 'threshold_disk_critical',
+    ];
+
     public function __construct()
     {
         $this->db = Database::getInstance();
@@ -108,8 +118,14 @@ class Server
         ];
     }
 
+    private function filterData(array $data): array
+    {
+        return array_intersect_key($data, array_flip($this->fillable));
+    }
+
     public function create(array $data): int
     {
+        $data = $this->filterData($data);
         if (!empty($data['ssh_password'])) {
             $data['ssh_password'] = $this->encryption->encrypt($data['ssh_password']);
         } else {
@@ -143,6 +159,7 @@ class Server
 
     public function update(int $id, array $data): bool
     {
+        $data = $this->filterData($data);
         if (array_key_exists('ssh_password', $data)) {
             if (!empty($data['ssh_password'])) {
                 $data['ssh_password'] = $this->encryption->encrypt($data['ssh_password']);

views/dashboard/index.php

@@ -165,7 +165,7 @@ $aggregatedMetrics = $aggregatedMetrics ?? [];
 
 <script src="https://cdn.jsdelivr.net/npm/chart.js@4.4.7/dist/chart.umd.min.js"></script>
 <script>
-const aggregatedData = <?= json_encode($aggregatedMetrics) ?>;
+const aggregatedData = <?= json_encode($aggregatedMetrics, JSON_HEX_TAG | JSON_UNESCAPED_SLASHES) ?>;
 
 let aggregatedChart = null;
 

views/layouts/main.php

@@ -132,7 +132,10 @@
                     </div>
                     <div class="user-menu">
                         <a href="/profile" title="Profile"><i class="fas fa-user-cog"></i></a>
-                        <a href="/logout" title="Logout"><i class="fas fa-sign-out-alt"></i></a>
+                        <form method="POST" action="/logout" class="logout-form" style="display:inline">
+                            <input type="hidden" name="_csrf_token" value="<?= $this->csrfToken() ?>">
+                            <button type="submit" title="Logout" style="background:none;border:none;color:inherit;cursor:pointer;padding:0;font:inherit;line-height:1"><i class="fas fa-sign-out-alt"></i></button>
+                        </form>
                     </div>
                 </div>
             </div>
@@ -407,7 +410,7 @@ function toggleServerList(e) {
                         const status = s.current_status === 'online' ? 'success' : s.current_status === 'offline' ? 'danger' : 'muted';
                         html += '<a href="/servers/' + s.id + '" class="nav-sub-item">'
                             + '<span class="status-dot-sm status-' + status + '"></span> '
-                            + s.name + '</a>';
+                            + escapeHtml(s.name) + '</a>';
                     });
                     sub.innerHTML = html;
                 } else {

views/servers/services.php

@@ -47,7 +47,7 @@ $totalPages = max(1, ceil(count($services) / $perPage));
 </div>
 
 <script>
-const allServices = <?= json_encode($services) ?>;
+const allServices = <?= json_encode($services, JSON_HEX_TAG | JSON_UNESCAPED_SLASHES) ?>;
 const perPage = <?= $perPage ?>;
 
 function applyFilters() {

views/servers/show.php

@@ -371,7 +371,7 @@ $canManage = $server_role === 'owner' || $server_role === 'manager';
 
 <script src="https://cdn.jsdelivr.net/npm/chart.js@4.4.7/dist/chart.umd.min.js"></script>
 <script>
-const historicalData = <?= json_encode($historicalMetrics) ?>;
+const historicalData = <?= json_encode($historicalMetrics, JSON_HEX_TAG | JSON_UNESCAPED_SLASHES) ?>;
 
 let metricsChart = null;