fix: address code review feedback — escaping, CSRF token, gmdate, regex config parsing

Agent-Logs-Url: https://github.com/GameServerPanel/GSP/sessions/d2560591-832a-44dc-bd98-baf5c3e26cd5 Co-authored-by: iaretechnician <2749183+iaretechnician@users.noreply.github.com>
2026-04-27 20:39:47 +00:00 · 2026-04-27 20:39:47 +00:00 · e016d78206
commit e016d78206
parent c488e89a45
3 changed files with 48 additions and 22 deletions
--- a/INSTALL_README.md
+++ b/INSTALL_README.md
@ -114,8 +114,8 @@ If you need to reinstall the panel (e.g. after a migration or reset):
 When the installer detects **existing tables** in the target database, it:

 1. Displays a warning: _"Existing database detected. A backup will be created before reinstall."_
-2. Creates a backup database named `panel_BAK`.
-   - If `panel_BAK` already exists, a timestamped name is used: `panel_BAK_YYYYMMDD_HHMMSS`.
+2. Creates a backup database named `{database_name}_BAK` (e.g. `panel_BAK` if your DB is `panel`).
+   - If `{database_name}_BAK` already exists, a timestamped name is used: `{database_name}_BAK_YYYYMMDD_HHMMSS`.
 3. Copies schema + data for every table into the backup database.
 4. Drops all tables from the target database.
 5. Proceeds with a fresh install.
@ -126,7 +126,7 @@ When the installer detects **existing tables** in the target database, it:
 -- Example restore of a single table
 INSERT INTO panel.gsp_users SELECT * FROM panel_BAK.gsp_users;

-- Or restore the full backup DB
+-- Or restore the full backup DB (replace panel_BAK with the actual backup name)
 mysqldump panel_BAK | mysql panel
 ```

--- a/check.php
+++ b/check.php
@ -125,7 +125,7 @@ $rows[] = [
    'section' => 'PHP Libraries',
    'name'    => 'PEAR',
    'status'  => $pear_path !== false ? 'ok' : 'warning',
-    'current' => $pear_path !== false ? h($pear_path) : 'Not found',
+    'current' => $pear_path !== false ? $pear_path : 'Not found',
    'fix'     => $pear_path !== false ? '' : 'sudo apt install php-pear -y',
    'notes'   => 'Used by some legacy OGP/GSP modules.',
 ];
@ -233,24 +233,32 @@ if (function_exists('apache_get_modules')) {
 // ── Optional DB connectivity test ────────────────────────────────────────────
 $config_path = CHECK_BASE . '/includes/config.inc.php';
 if (is_readable($config_path)) {
-    // Extract only the variables we need without executing arbitrary code.
-    // We use a limited include inside an isolated function scope.
-    $db_host = $db_port = $db_user = $db_pass = $db_name = null;
-    (static function () use ($config_path, &$db_host, &$db_port, &$db_user, &$db_pass, &$db_name): void {
-        @include $config_path;
-    })();
+    // Extract credentials using regex instead of executing the config file,
+    // to avoid running arbitrary PHP code from the config.
+    $raw = file_get_contents($config_path);
+    $cfg = [];
+    foreach (['db_host', 'db_port', 'db_user', 'db_pass', 'db_name'] as $var) {
+        if ($raw !== false && preg_match('/\$' . $var . '\s*=\s*"([^"]*)"/', $raw, $m)) {
+            $cfg[$var] = $m[1];
+        }
+    }

-    if ($db_host !== null && $db_user !== null && $db_name !== null) {
-        $db_port_int = (int)($db_port ?? 3306);
-        $conn = @mysqli_connect($db_host, $db_user, $db_pass ?? '', $db_name, $db_port_int);
+    $db_host_cfg = $cfg['db_host'] ?? null;
+    $db_user_cfg = $cfg['db_user'] ?? null;
+    $db_pass_cfg = $cfg['db_pass'] ?? null;
+    $db_name_cfg = $cfg['db_name'] ?? null;
+    $db_port_cfg = isset($cfg['db_port']) ? (int)$cfg['db_port'] : 3306;
+
+    if ($db_host_cfg !== null && $db_user_cfg !== null && $db_name_cfg !== null) {
+        $conn = @mysqli_connect($db_host_cfg, $db_user_cfg, $db_pass_cfg ?? '', $db_name_cfg, $db_port_cfg);
        if ($conn) {
            $db_status  = 'ok';
-            $db_current = 'Connected to ' . h($db_host) . ':' . $db_port_int . ' / ' . h($db_name);
+            $db_current = 'Connected to ' . $db_host_cfg . ':' . $db_port_cfg . ' / ' . $db_name_cfg;
            $db_fix     = '';
            mysqli_close($conn);
        } else {
            $db_status  = 'warning';
-            $db_current = 'Connection failed — ' . h(mysqli_connect_error());
+            $db_current = 'Connection failed — ' . (mysqli_connect_error() ?? 'unknown error');
            $db_fix     = 'Check credentials in includes/config.inc.php';
        }

@ -260,7 +268,7 @@ if (is_readable($config_path)) {
            'status'  => $db_status,
            'current' => $db_current,
            'fix'     => $db_fix,
-            'notes'   => 'Host: ' . h($db_host) . ' | Port: ' . $db_port_int . ' | DB: ' . h($db_name) . ' | User: ' . h($db_user ?? ''),
+            'notes'   => 'Host: ' . $db_host_cfg . ' | Port: ' . $db_port_cfg . ' | DB: ' . $db_name_cfg . ' | User: ' . $db_user_cfg,
        ];
    } else {
        $rows[] = [
@ -361,6 +369,7 @@ foreach ($rows as $r) {
        .note-box { background: #1a2a1a; border: 1px solid #2e5a2e; border-radius: 8px; padding: 14px 18px; margin-bottom: 20px; color: #9ddd9d; font-size: 13px; }
        .note-box strong { color: #7ddb7d; }
        footer { text-align: center; color: #555; font-size: 11px; margin-top: 30px; }
+
    </style>
 </head>
 <body>
@ -435,7 +444,7 @@ foreach ($rows as $r) {
                        <?= h($row['status']) ?>
                    </span>
                </td>
-                <td class="td-current"><?= $row['current'] /* already escaped where built */ ?></td>
+                <td class="td-current"><?= h($row['current']) ?></td>
                <td class="td-fix">
                    <?php if (!empty($row['fix'])): ?>
                        <?php if (strpos($row['fix'], "\n") !== false): ?>
@ -469,7 +478,7 @@ sudo systemctl restart apache2'
        </tbody>
    </table>

-    <footer>GSP / WDS Dependency Checker — safe to run at any time &mdash; generated at <?= h(date('Y-m-d H:i:s')) ?> UTC</footer>
+    <footer>GSP / WDS Dependency Checker — safe to run at any time &mdash; generated at <?= h(gmdate('Y-m-d H:i:s')) ?> UTC</footer>
 </div>
 </body>
 </html>
--- a/install.php
+++ b/install.php
@ -428,7 +428,7 @@ function gsp_backup_existing_db($db, $db_host, $db_user, $db_pass, $db_name, $db
    }

    $table_count = count($tables_result);
-    echo "<p class='note' style='color:#b35900;font-weight:bold;'>&#x26A0; Existing database detected ({$table_count} table(s)). Creating backup before reinstall&hellip;</p>";
+    echo "<p class='note' style='color:#b35900;font-weight:bold;'>&#x26A0; Existing database detected (" . htmlspecialchars((string)$table_count, ENT_QUOTES, 'UTF-8') . " table(s)). Creating backup before reinstall&hellip;</p>";

    // Determine backup DB name
    $backup_name = $db_name . '_BAK';
@ -513,22 +513,38 @@ function gsp_disable_installer() {
 * To restore the installer:
 *   cp install.php.bak install.php
 *
- * Or via the button below (admin action — currently unprotected; remove this
- * file when you no longer need reinstall capability).
+ * Or via the button below (admin action — requires a one-time restore token).
 */
+session_start();
+
+// Generate a one-time CSRF token if not already set
+if (empty($_SESSION['gsp_restore_token'])) {
+    $_SESSION['gsp_restore_token'] = bin2hex(random_bytes(16));
+}

 if (isset($_POST['restore_installer'])) {
+    // Validate CSRF token
+    $submitted = $_POST['gsp_restore_token'] ?? '';
+    if (!hash_equals($_SESSION['gsp_restore_token'], $submitted)) {
+        http_response_code(403);
+        die('<p style="color:red">Invalid or expired restore token. Please reload the page and try again.</p>');
+    }
+
    $bak = __DIR__ . '/install.php.bak';
    if (!is_readable($bak)) {
        die('<p style="color:red">install.php.bak not found. Restore manually.</p>');
    }
    $content = file_get_contents($bak);
-    if (file_put_contents(__FILE__, $content) === false) {
+    if ($content === false || file_put_contents(__FILE__, $content) === false) {
        die('<p style="color:red">Could not overwrite install.php. Check file permissions.</p>');
    }
+    // Invalidate the token after use
+    unset($_SESSION['gsp_restore_token']);
    header('Location: install.php');
    exit;
 }
+
+$token = htmlspecialchars($_SESSION['gsp_restore_token'], ENT_QUOTES, 'UTF-8');
 ?>
 <!DOCTYPE html>
 <html lang="en">
@ -552,6 +568,7 @@ code { background:#0d0d22; color:#aaf; padding:2px 8px; border-radius:4px; font-
    <p>The GSP installer has been disabled after a successful installation to prevent accidental re-runs.</p>
    <p>The original installer is preserved at <code>install.php.bak</code>.</p>
    <form method="post">
+        <input type="hidden" name="gsp_restore_token" value="<?= $token ?>">
        <button type="submit" name="restore_installer" class="btn" onclick="return confirm('Restore the full installer? Only do this if you intend to reinstall the panel.');">&#x21BA; Restore &amp; Re-run Installer</button>
    </form>
    <p class="note">