From 580702c5bc263598fc2c1694a477d0ae85faac33 Mon Sep 17 00:00:00 2001 From: Catalin Novgorodschi Date: Sun, 14 Jun 2026 10:22:18 +0300 Subject: [PATCH] Some security for admin.php Added some security and some style change --- Admin/admin.php | 1011 ++++++++++++++++++++++++++++------------------- 1 file changed, 609 insertions(+), 402 deletions(-) diff --git a/Admin/admin.php b/Admin/admin.php index 19f1a1f0..54eb60a7 100644 --- a/Admin/admin.php +++ b/Admin/admin.php @@ -17,19 +17,156 @@ ## ## ################################################################################# -session_start(); +// ─── SESSION ───────────────────────────────────────────────────────────────── +// Harden session cookie before session_start() — has no effect after. +if (session_status() === PHP_SESSION_NONE) { + session_set_cookie_params([ + 'lifetime' => 0, + 'path' => '/', + 'secure' => isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off', + 'httponly' => true, + 'samesite' => 'Strict', + ]); + session_start(); +} + +// ─── CSRF TOKEN ─────────────────────────────────────────────────────────────── +// Generat o singură dată per sesiune și stocat în $_SESSION. +// Toate request-urile POST trebuie să trimită acest token în câmpul _csrf_token. +if (empty($_SESSION['_csrf_token'])) { + $_SESSION['_csrf_token'] = bin2hex(random_bytes(32)); +} + +// ─── CORE INCLUDES ─────────────────────────────────────────────────────────── include_once("../GameEngine/config.php"); include_once("../GameEngine/Database.php"); -include_once ("../GameEngine/Lang/" . LANG . ".php"); +include_once("../GameEngine/Lang/" . LANG . ".php"); include_once("../GameEngine/Admin/database.php"); include_once("../GameEngine/Data/buidata.php"); include_once("../GameEngine/Artifacts.php"); -$subpage = 'Login'; +// ─── SECURITY HELPERS ──────────────────────────────────────────────────────── + +/** + * Return a sanitised integer from a superglobal key, or null if missing/invalid. + * Replaces direct (int) casts on $_GET inside switch — ensures 0 is treated as + * absent (IDs are always >= 1 in TravianZ). + */ +function admin_input_id(array $source, string $key): ?int +{ + if (!isset($source[$key]) || !ctype_digit((string)$source[$key])) { + return null; + } + $v = (int)$source[$key]; + return $v > 0 ? $v : null; +} + +/** + * HTML-escape a value for safe output inside HTML attributes or text nodes. + */ +function e(string $value): string +{ + return htmlspecialchars($value, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'); +} + +/** + * Whitelist-validate the ?p= parameter. + * Returns the validated page string, or '' if not in the whitelist. + * + * SECURITY: This is the primary defence against path-traversal in the + * include('Templates/'.$p.'.tpl') call below. Only values present in this + * array are ever passed to include(). + */ +function admin_validated_page(string $raw): string +{ + static $whitelist = [ + 'server_info', 'online', 'notregistered', 'inactive', 'report', + 'message', 'massmessage', 'sysmessage', 'map', 'map_tile', 'natars', + 'search', 'ban', 'maintenance', 'cleanban', 'gold', 'usergold', + 'maintenenceResetGold', 'delmedal', 'delallymedal', 'givePlus', + 'maintenenceResetPlus', 'givePlusRes', 'maintenenceResetPlusBonus', + 'addUsers', 'users', 'admin_log', 'config', 'debug_log', + 'editServerSet', 'editPlusSet', 'editLogSet', 'editNewsboxSet', + 'editExtraSet', 'editAdminInfo', 'resetServer', 'player', 'editUser', + 'deletion', 'Newmessage', 'editPlus', 'editSitter', 'editOverall', + 'editWeek', 'userlogin', 'userillegallog', 'editHero', 'editAdditional', + 'village', 'editResources', 'addTroops', 'addABTroops', 'editVillage', + 'villagelog', 'techlog', 'msg', + ]; + + return in_array($raw, $whitelist, true) ? $raw : ''; +} + +/** + * Returnează token-ul CSRF curent ca string hex. + * Folosit pentru injectare în câmpuri ascunse sau header-e AJAX. + */ +function csrf_token(): string +{ + return $_SESSION['_csrf_token'] ?? ''; +} + +/** + * Emite un gata de pus în orice
POST din template-uri. + * Exemplu de utilizare în .tpl: + */ +function csrf_field(): string +{ + return ''; +} + +/** + * Verifică token-ul CSRF dintr-un request POST. + * Oprește execuția cu HTTP 403 dacă token-ul lipsește sau nu se potrivește. + * Apelată automat pe orice $_POST — nu trebuie apelată manual în template-uri. + * + * Folosim hash_equals() în loc de === pentru a preveni timing attacks. + */ +function csrf_verify(): void +{ + $submitted = isset($_POST['_csrf_token']) ? (string)$_POST['_csrf_token'] : ''; + $stored = csrf_token(); + + if ($stored === '' || !hash_equals($stored, $submitted)) { + http_response_code(403); + // Mesaj generic — nu dezvăluie detalii despre mecanism + die('

403 Forbidden

Invalid or missing security token. Please go back and try again.

'); + } +} + +/** + * Look up a user row by ID using a prepared statement. + * Replaces the two raw mysqli_query() calls for userlogin / userillegallog. + * + * Returns the associative row, or null on failure / not found. + */ +function admin_get_user_by_id(int $uid): ?array +{ + $link = $GLOBALS['link']; + $stmt = mysqli_prepare($link, "SELECT * FROM `" . TB_PREFIX . "users` WHERE `id` = ?"); + if (!$stmt) { + return null; + } + mysqli_stmt_bind_param($stmt, 'i', $uid); + mysqli_stmt_execute($stmt); + $result = mysqli_stmt_get_result($stmt); + $row = $result ? mysqli_fetch_assoc($result) : null; + mysqli_stmt_close($stmt); + return $row ?: null; +} + +// ─── PAGE ROUTING ───────────────────────────────────────────────────────────── +// Read and whitelist the ?p= parameter once; all branching below uses $page. +$rawPage = isset($_GET['p']) ? trim((string)$_GET['p']) : ''; +$page = admin_validated_page($rawPage); + +$subpage = 'Login'; $not_include_mootools_js = false; -if (!empty($_GET['p'])) { - switch ($_GET['p']) { +if ($page !== '') { + switch ($page) { + + // ── Simple label-only pages ────────────────────────────────────────── case 'server_info': $subpage = 'Server Info'; break; @@ -49,28 +186,37 @@ if (!empty($_GET['p'])) { case 'report': $subpage = 'Players Report'; break; - + case 'message': + // NOTE: original code had this case duplicated (second occurrence + // overrode with 'Search IGMs/Reports'). The first definition + // ('Players Message') is intentional for the ?p=message route. + // The 'Search IGMs/Reports' label belongs to ?p=search sub-section + // which is already covered by the search template include logic. $subpage = 'Players Message'; break; - - case 'massmessage': - $subpage = 'Mass Message'; - break; - - case 'sysmessage': - $subpage = 'System Message'; - break; + + case 'msg': + $subpage = 'Search IGMs/Reports'; + break; + + case 'massmessage': + $subpage = 'Mass Message'; + break; + + case 'sysmessage': + $subpage = 'System Message'; + break; case 'map': $subpage = 'Map'; break; - - case 'map_tile': - $subpage = 'Map Tile'; - $not_include_mootools_js = true; - break; - + + case 'map_tile': + $subpage = 'Map Tile'; + $not_include_mootools_js = true; + break; + case 'natars': $subpage = 'Natars Management'; break; @@ -79,10 +225,6 @@ if (!empty($_GET['p'])) { $subpage = 'General Search'; break; - case 'message': - $subpage = 'Search IGMs/Reports'; - break; - case 'ban': $subpage = 'Ban/Unban Players'; break; @@ -179,191 +321,220 @@ if (!empty($_GET['p'])) { $subpage = 'Server Resetting'; break; + // ── User-context pages (require a valid ?uid=) ─────────────────────── case 'player': - if (!empty($_GET['uid'])) { - $displayarray = $database->getUserArray($_GET['uid'],1); - $user=$displayarray; - $subpage = 'Player Details ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $displayarray = $database->getUserArray($uid, 1); + $user = $displayarray; + $subpage = 'Player Details (' . e($user['username']) . ')'; } else { $subpage = 'Player Details (no player)'; } break; case 'editUser': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Edit Player ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Edit Player (' . e($user['username']) . ')'; } else { $subpage = 'Edit Player (no player)'; } break; case 'deletion': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Delete Player ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Delete Player (' . e($user['username']) . ')'; } else { $subpage = 'Delete Player (no player)'; } break; case 'Newmessage': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Compose Message ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Compose Message (' . e($user['username']) . ')'; } else { $subpage = 'Compose Message'; } break; case 'editPlus': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Edit Plus & Resources ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Edit Plus & Resources (' . e($user['username']) . ')'; } else { $subpage = 'Edit Plus & Resources'; } break; case 'editSitter': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Edit Sitters ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Edit Sitters (' . e($user['username']) . ')'; } else { - $subpage = 'Edit Sitters '; + $subpage = 'Edit Sitters'; } break; case 'editOverall': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Edit Off & Def ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Edit Off & Def (' . e($user['username']) . ')'; } else { $subpage = 'Edit Off & Def'; } break; case 'editWeek': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Edit Weekly Off & Def ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Edit Weekly Off & Def (' . e($user['username']) . ')'; } else { $subpage = 'Edit Weekly Off & Def'; } break; case 'userlogin': - if (!empty($_GET['uid'])) { - $player = mysqli_fetch_assoc(mysqli_query($GLOBALS["link"], "SELECT * FROM ".TB_PREFIX."users WHERE id = ".(int) $_GET['uid'])); - $subpage = 'User Logins ('.$player['username'].')'; + // SECURITY FIX: was raw mysqli_query with direct $_GET interpolation. + // Now uses admin_get_user_by_id() which internally uses a prepared statement. + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $player = admin_get_user_by_id($uid); + $subpage = $player + ? 'User Logins (' . e($player['username']) . ')' + : 'User Logins (player not found)'; } else { $subpage = 'User Logins (no player)'; } break; case 'userillegallog': - if (!empty($_GET['uid'])) { - $player = mysqli_fetch_assoc(mysqli_query($GLOBALS["link"], "SELECT * FROM ".TB_PREFIX."users WHERE id = ".(int) $_GET['uid'])); - $subpage = 'User Illegals Log ('.$player['username'].')'; + // SECURITY FIX: same as userlogin above. + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $player = admin_get_user_by_id($uid); + $subpage = $player + ? 'User Illegals Log (' . e($player['username']) . ')' + : 'User Illegals Log (player not found)'; } else { $subpage = 'User Illegals Log (no player)'; } break; case 'editHero': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Edit Hero ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Edit Hero (' . e($user['username']) . ')'; } else { $subpage = 'Edit Hero'; } break; case 'editAdditional': - if (!empty($_GET['uid'])) { - $user = $database->getUserArray($_GET['uid'],1); - $subpage = 'Edit Additional Info ('.$user['username'].')'; + $uid = admin_input_id($_GET, 'uid'); + if ($uid !== null) { + $user = $database->getUserArray($uid, 1); + $subpage = 'Edit Additional Info (' . e($user['username']) . ')'; } else { $subpage = 'Edit Additional Info'; } break; - case 'village': - if (!empty($_GET['did'])) { - $did = (int)$_GET['did']; - $village = $database->getVillage($did); - if ($village) { - $user = $database->getUserArray($village['owner'], 1); - $subpage = 'Edit Village ('.$village['name'].' » '.($user['username'] ?? '?').')'; - } else { - $subpage = 'Edit Village (ID '.$did.' not found)'; - $village = null; - } - } else { - $subpage = 'Edit Village (no village)'; - } - break; + // ── Village-context pages (require a valid ?did=) ──────────────────── + case 'village': + $did = admin_input_id($_GET, 'did'); + if ($did !== null) { + $village = $database->getVillage($did); + if ($village) { + $user = $database->getUserArray($village['owner'], 1); + $subpage = 'Edit Village (' . e($village['name']) . ' » ' . e($user['username'] ?? '?') . ')'; + } else { + $subpage = 'Edit Village (ID ' . $did . ' not found)'; + $village = null; + } + } else { + $subpage = 'Edit Village (no village)'; + } + break; - case 'editResources': - if (!empty($_GET['did'])) { - $village = $database->getVillage($_GET['did']); - if ($village) { - $user = $database->getUserArray($village['owner'], 1); - $subpage = 'Edit Resources ('.$village['name'].' » '.$user['username'].')'; - } else { - $subpage = 'Edit Resources (ID '.$did.' not found)'; - $village = null; - } - } else { - $subpage = 'Edit Resources (no village)'; - } - break; + case 'editResources': + $did = admin_input_id($_GET, 'did'); + if ($did !== null) { + $village = $database->getVillage($did); + if ($village) { + $user = $database->getUserArray($village['owner'], 1); + $subpage = 'Edit Resources (' . e($village['name']) . ' » ' . e($user['username']) . ')'; + } else { + // BUGFIX: original used $did which was only set in 'village' case, + // causing an undefined variable notice here. Now always defined above. + $subpage = 'Edit Resources (ID ' . $did . ' not found)'; + $village = null; + } + } else { + $subpage = 'Edit Resources (no village)'; + } + break; case 'addTroops': - if (!empty($_GET['did'])) { - $village = $database->getVillage($_GET['did']); - $user = $database->getUserArray($village['owner'], 1); - $subpage = 'Edit Troops ('.$village['name'].' » '.$user['username'].')'; + $did = admin_input_id($_GET, 'did'); + if ($did !== null) { + $village = $database->getVillage($did); + $user = $database->getUserArray($village['owner'], 1); + $subpage = 'Edit Troops (' . e($village['name']) . ' » ' . e($user['username']) . ')'; } else { $subpage = 'Edit Troops (no village)'; } break; case 'addABTroops': - if (!empty($_GET['did'])) { - $village = $database->getVillage($_GET['did']); - $user = $database->getUserArray($village['owner'],1); - $subpage = 'Upgrade Troops ('.$village['name'].' » '.$user['username'].')'; + $did = admin_input_id($_GET, 'did'); + if ($did !== null) { + $village = $database->getVillage($did); + $user = $database->getUserArray($village['owner'], 1); + $subpage = 'Upgrade Troops (' . e($village['name']) . ' » ' . e($user['username']) . ')'; } else { $subpage = 'Upgrade Troops (no village)'; } break; case 'editVillage': - if (!empty($_GET['did'])) { - $village = $database->getVillage($_GET['did']); - $user = $database->getUserArray($village['owner'],1); - $subpage = 'Edit Village ('.$village['name'].' » '.$user['username'].')'; + $did = admin_input_id($_GET, 'did'); + if ($did !== null) { + $village = $database->getVillage($did); + $user = $database->getUserArray($village['owner'], 1); + $subpage = 'Edit Village (' . e($village['name']) . ' » ' . e($user['username']) . ')'; } else { $subpage = 'Edit Village (no village)'; } break; case 'villagelog': - if (!empty($_GET['did'])) { - $village = $database->getVillage($_GET['did']); - $user = $database->getUserArray($village['owner'],1); - $subpage = 'Build Log ('.$village['name'].' » '.$user['username'].')'; + $did = admin_input_id($_GET, 'did'); + if ($did !== null) { + $village = $database->getVillage($did); + $user = $database->getUserArray($village['owner'], 1); + $subpage = 'Build Log (' . e($village['name']) . ' » ' . e($user['username']) . ')'; } else { $subpage = 'Build Log (no village)'; } break; case 'techlog': - if (!empty($_GET['did'])) { - $village = $database->getVillage($_GET['did']); - $user = $database->getUserArray($village['owner'],1); - $subpage = 'Research Log ('.$village['name'].' » '.$user['username'].')'; + $did = admin_input_id($_GET, 'did'); + if ($did !== null) { + $village = $database->getVillage($did); + $user = $database->getUserArray($village['owner'], 1); + $subpage = 'Research Log (' . e($village['name']) . ' » ' . e($user['username']) . ')'; } else { $subpage = 'Research Log (no village)'; } @@ -371,25 +542,42 @@ if (!empty($_GET['p'])) { } } -?> - +// ─── SECURITY HEADERS ───────────────────────────────────────────────────────── +// Send headers before ANY output. These protect against common web attacks. +// Intentionally NOT using header_remove() to avoid stripping headers set by +// other TravianZ bootstrap code — we only add, never remove. +if (!headers_sent()) { + header('X-Frame-Options: DENY'); + header('X-Content-Type-Options: nosniff'); + header('Referrer-Policy: strict-origin-when-cross-origin'); + header("Content-Security-Policy: default-src 'self'; " + . "script-src 'self' 'unsafe-inline' https://ajax.googleapis.com; " + . "style-src 'self' 'unsafe-inline'; " + . "img-src 'self' data:; " + . "font-src 'self'; " + . "connect-src 'self'; " + . "frame-ancestors 'none';"); +} + +?> - - - Admin Panel - <?php echo $subpage; ?> - - - - - - - - - - - - - + + + Admin Panel - <?php echo e($subpage); ?> + + + + + + + + + + + + + + - - + + + - - - + init_local(); -
-
-
- -
-

TravianZ Admin Panel

- • v05.03.2026 -
-
-
- CheckLogin()){ ?> - getUserField($_SESSION['id'], 'username', 0); - $adminAccess = $database->getUserField($_SESSION['id'], 'access', 0); - $rank = $adminAccess == 9 ? 'Admin' : ($adminAccess == 8 ? 'MH' : 'User'); - ?> - Logged: () - Logout - - Not Logged in - -
-
-
-
-
-
- -
-
- CheckLogin()) - { - if($_POST || $_GET) - { - $p = $_GET['p'] ?? ''; - if($p && $p != "search") - { - $filename = 'Templates/'.$p.'.tpl'; - if(file_exists($filename)) include($filename); - else include('Templates/404.tpl'); - } - else include('Templates/search.tpl'); - - if(isset($_POST['p']) && isset($_POST['s']) && $_POST['p'] && $_POST['s']) - { - $filename = 'Templates/results_'.$_POST['p'].'.tpl'; - if(file_exists($filename)) include($filename); - else include('Templates/404.tpl'); + function getMouseCoords(e) { + var coords = {}; + e = e || window.event; + if (e.pageX || e.pageY) { + coords.x = e.pageX; + coords.y = e.pageY; + } else if (e.clientX || e.clientY) { + coords.x = e.clientX + document.body.scrollLeft + document.documentElement.scrollLeft; + coords.y = e.clientY + document.body.scrollTop + document.documentElement.scrollTop; } + return coords; } - else include('Templates/home.tpl'); - } - else include('Templates/login.tpl'); -?> -
-
-
-
-
-
- + + function med_mouseMoveHandler(e, desc_string) { + var coords = getMouseCoords(e); + med_showDescription(coords, desc_string); + } + + function med_closeDescription() { + document.getElementById('medal_mouseover').className = 'hide'; + } + + function init_local() { + med_init(); + } + + function med_init() { + var layer = document.createElement('div'); + layer.id = 'medal_mouseover'; + layer.className = 'hide'; + document.body.appendChild(layer); + } + + function med_showDescription(coords, desc_string) { + var layer = document.getElementById('medal_mouseover'); + layer.style.top = (coords.y + 25) + 'px'; + layer.style.left = (coords.x - 20) + 'px'; + layer.className = ''; + layer.innerHTML = desc_string; + } + + + + + + +
+
+
+ +
+

TravianZ Admin Panel

+ • v14.06.2026 +
+
+
+ CheckLogin()) { ?> + getUserField($_SESSION['id'], 'username', 0); + $adminAccess = $database->getUserField($_SESSION['id'], 'access', 0); + $rank = $adminAccess == 9 ? 'Admin' : ($adminAccess == 8 ? 'MH' : 'User'); + ?> + Logged: + () + Logout + + Not Logged in + +
+
+
+ +
+ +
+
+ + +
+
+ CheckLogin()) { + // CSRF: verifică token-ul pe ORICE request POST înainte de a + // include orice template. GET-urile nu modifică starea serverului + // (sunt doar citiri), deci nu necesită verificare CSRF. + if ($_POST) { + csrf_verify(); + } + + if ($_POST || $_GET) { + // SECURITY: $page is already whitelist-validated above. + // Direct string concat with include() is now safe. + if ($page !== '' && $page !== 'search') { + $filename = 'Templates/' . $page . '.tpl'; + if (file_exists($filename)) { + include($filename); + } else { + include('Templates/404.tpl'); + } + } else { + include('Templates/search.tpl'); + } + + // Handle POST-based results template. + $postPage = isset($_POST['p']) ? trim((string)$_POST['p']) : ''; + $postSub = isset($_POST['s']) ? trim((string)$_POST['s']) : ''; + $postPage = admin_validated_page($postPage); // whitelist POST too + if ($postPage !== '' && $postSub !== '') { + $filename = 'Templates/results_' . $postPage . '.tpl'; + if (file_exists($filename)) { + include($filename); + } else { + include('Templates/404.tpl'); + } + } + } else { + include('Templates/home.tpl'); + } + } else { + include('Templates/login.tpl'); + } + ?> +
+
+ +
+
+
+
+ + \ No newline at end of file