From 790d85a88a2b4094eba1c1d0c0554d0ef7bd4b96 Mon Sep 17 00:00:00 2001 From: Slawomir Boczek Date: Sat, 11 Nov 2023 10:57:57 +0100 Subject: [PATCH] CSRF Protection (#235) * Fix alert class name * feature: csrf protection * Cosmetics * Fix token generate * Admin Panel: changelogs csrf protection * news/id route * Refactor admin newses + add csrf * Use admin.links instead * Admin panel: Pages csrf * Menus: better csrf + add success message on reset colors * Plugins csrf * Move definitions * add info function, same as note($message) * Update mailer.php * Fix new page/news links * clear_cache & maintenance csrf * Formatting * Fix news type * Fix changelog link * Add new changelog link * More info to confirm dialog * This is always true --- admin/pages/accounts.php | 8 +- admin/pages/changelog.php | 49 ++--- admin/pages/dashboard.php | 6 +- admin/pages/login.php | 2 + admin/pages/mailer.php | 4 +- admin/pages/mass_account.php | 2 + admin/pages/mass_teleport.php | 2 + admin/pages/menus.php | 21 +- admin/pages/modules/templates/web_status.twig | 46 +++-- admin/pages/news.php | 40 ++-- admin/pages/notepad.php | 2 + admin/pages/pages.php | 193 ++++-------------- admin/pages/players.php | 8 +- admin/pages/plugins.php | 17 +- admin/tools/settings_save.php | 2 + common.php | 7 + system/functions.php | 35 +++- system/init.php | 9 + system/libs/changelog.php | 1 + system/logout.php | 5 + system/pages/account/change_email.php | 8 +- system/pages/faq.php | 4 +- system/pages/news.php | 25 +-- system/router.php | 5 +- system/routes.php | 1 + system/settings.php | 6 + system/src/Admin/Pages.php | 134 ++++++++++++ system/src/CsrfToken.php | 95 +++++++++ .../templates/account.back_button.html.twig | 1 + .../account.change_comment.html.twig | 2 + .../templates/account.change_info.html.twig | 4 +- .../templates/account.change_mail.html.twig | 2 + .../templates/account.change_name.html.twig | 2 + .../account.change_password.html.twig | 2 + system/templates/account.change_sex.html.twig | 2 + system/templates/account.create.html.twig | 1 + .../account.create_character.html.twig | 2 + .../account.delete_character.html.twig | 4 +- ...ccount.generate_new_recovery_key.html.twig | 2 + .../account.generate_recovery_key.html.twig | 4 +- system/templates/account.login.html.twig | 4 +- system/templates/account.lost.form.html.twig | 5 +- system/templates/account.management.html.twig | 8 + system/templates/admin-bar.html.twig | 9 +- .../templates/admin.changelog.form.html.twig | 2 + system/templates/admin.changelog.html.twig | 48 ++++- system/templates/admin.links.html.twig | 22 ++ system/templates/admin.login.html.twig | 1 + system/templates/admin.mailer.html.twig | 1 + system/templates/admin.menus.form.html.twig | 1 + system/templates/admin.news.form.html.twig | 14 +- system/templates/admin.news.html.twig | 136 +----------- system/templates/admin.news.table.html.twig | 64 ++++++ system/templates/admin.notepad.html.twig | 1 + system/templates/admin.pages.form.html.twig | 4 +- system/templates/admin.pages.html.twig | 36 +++- system/templates/admin.pages.links.html.twig | 14 -- system/templates/admin.plugins.form.html.twig | 1 + system/templates/admin.plugins.html.twig | 24 ++- system/templates/admin.settings.html.twig | 6 + .../templates/admin.tools.account.html.twig | 3 + system/templates/characters.html.twig | 4 +- system/templates/faq.form.html.twig | 9 +- system/templates/forum.add_board.html.twig | 3 +- system/templates/forum.edit_post.html.twig | 3 +- system/templates/forum.move_thread.html.twig | 3 +- system/templates/forum.new_post.html.twig | 1 + system/templates/forum.new_thread.html.twig | 3 +- system/templates/gallery.form.html.twig | 3 +- .../templates/guilds.accept_invite.html.twig | 4 +- system/templates/guilds.back_button.html.twig | 3 +- .../guilds.change_description.html.twig | 2 + system/templates/guilds.change_logo.html.twig | 2 + system/templates/guilds.change_motd.html.twig | 2 + system/templates/guilds.change_rank.html.twig | 4 +- system/templates/guilds.create.html.twig | 4 +- .../templates/guilds.create.success.html.twig | 3 +- .../templates/guilds.delete_guild.html.twig | 4 +- .../templates/guilds.delete_invite.html.twig | 4 +- system/templates/guilds.invite.html.twig | 3 +- system/templates/guilds.kick_player.html.twig | 4 +- system/templates/guilds.leave_guild.html.twig | 4 +- system/templates/guilds.list.html.twig | 12 +- system/templates/guilds.manager.html.twig | 5 +- .../guilds.pass_leadership.html.twig | 4 +- system/templates/guilds.view.html.twig | 7 + system/templates/templates.header.html.twig | 2 + system/twig.php | 11 + .../tibiacom/news.featured_article.html.twig | 11 +- 89 files changed, 789 insertions(+), 504 deletions(-) create mode 100644 system/src/Admin/Pages.php create mode 100644 system/src/CsrfToken.php create mode 100644 system/templates/admin.links.html.twig create mode 100644 system/templates/admin.news.table.html.twig delete mode 100644 system/templates/admin.pages.links.html.twig diff --git a/admin/pages/accounts.php b/admin/pages/accounts.php index 214b82f017..4d410f11d9 100644 --- a/admin/pages/accounts.php +++ b/admin/pages/accounts.php @@ -13,6 +13,9 @@ defined('MYAAC') or die('Direct access not allowed!'); $title = 'Account editor'; + +csrfProtect(); + $admin_base = ADMIN_URL . '?p=accounts'; $use_datatable = true; @@ -82,7 +85,7 @@ $account = new OTS_Account(); $account->load($id); - if (isset($account, $_POST['save']) && $account->isLoaded()) { + if (isset($_POST['save']) && $account->isLoaded()) { $error = false; $_error = ''; @@ -289,6 +292,7 @@
+
@@ -581,6 +585,7 @@ class="form-check-input"/>
+
@@ -590,6 +595,7 @@ class="form-check-input"/>
+
diff --git a/admin/pages/changelog.php b/admin/pages/changelog.php index 3d5cad6464..ae2fd7b03e 100644 --- a/admin/pages/changelog.php +++ b/admin/pages/changelog.php @@ -13,30 +13,29 @@ defined('MYAAC') or die('Direct access not allowed!'); +$title = 'Changelog'; + +csrfProtect(); + if (!hasFlag(FLAG_CONTENT_PAGES) && !superAdmin()) { echo 'Access denied.'; return; } -$title = 'Changelog'; $use_datatable = true; const CL_LIMIT = 600; // maximum changelog body length -?> - - -orderBy('group_id', POT::ORDER_DESC); $twig->display('admin.changelog.form.html.twig', array( 'action' => $action, - 'cl_link_form' => constant('ADMIN_URL').'?p=changelog&action=' . ($action == 'edit' ? 'edit' : 'new'), + 'cl_link_form' => constant('ADMIN_URL').'?p=changelog', 'cl_id' => $id ?? null, 'body' => isset($body) ? escapeHtml($body) : '', 'create_date' => $create_date ?? '', @@ -128,15 +129,3 @@ $twig->display('admin.changelog.html.twig', array( 'changelogs' => $changelogs, )); - -?> - diff --git a/admin/pages/dashboard.php b/admin/pages/dashboard.php index e24b98ad22..734304565e 100644 --- a/admin/pages/dashboard.php +++ b/admin/pages/dashboard.php @@ -10,7 +10,9 @@ defined('MYAAC') or die('Direct access not allowed!'); $title = 'Dashboard'; -if (isset($_GET['clear_cache'])) { +csrfProtect(); + +if (isset($_POST['clear_cache'])) { if (clearCache()) { success('Cache cleared.'); } else { @@ -18,7 +20,7 @@ } } -if (isset($_GET['maintenance'])) { +if (isset($_POST['maintenance'])) { $message = (!empty($_POST['message']) ? $_POST['message'] : null); $_status = (isset($_POST['status']) && $_POST['status'] == 'true'); $_status = ($_status ? '0' : '1'); diff --git a/admin/pages/login.php b/admin/pages/login.php index 8bb25f3624..eb6466d312 100644 --- a/admin/pages/login.php +++ b/admin/pages/login.php @@ -10,6 +10,8 @@ defined('MYAAC') or die('Direct access not allowed!'); $title = 'Login'; +csrfProtect(); + require PAGES . 'account/login.php'; if ($logged) { header('Location: ' . (admin() ? ADMIN_URL : BASE_URL)); diff --git a/admin/pages/mailer.php b/admin/pages/mailer.php index 732b74615f..d9cf8888b2 100644 --- a/admin/pages/mailer.php +++ b/admin/pages/mailer.php @@ -10,6 +10,8 @@ defined('MYAAC') or die('Direct access not allowed!'); $title = 'Mailer'; +csrfProtect(); + if (!hasFlag(FLAG_CONTENT_MAILER) && !superAdmin()) { echo 'Access denied.'; return; @@ -20,7 +22,7 @@ return; } -$mail_to = isset($_REQUEST['mail_to']) ? stripslashes(trim($_REQUEST['mail_to'])) : null; +$mail_to = isset($_POST['mail_to']) ? stripslashes(trim($_POST['mail_to'])) : null; $mail_subject = isset($_POST['mail_subject']) ? stripslashes($_POST['mail_subject']) : null; $mail_content = isset($_POST['mail_content']) ? stripslashes($_POST['mail_content']) : null; diff --git a/admin/pages/mass_account.php b/admin/pages/mass_account.php index 63bec54cf4..549310a54c 100644 --- a/admin/pages/mass_account.php +++ b/admin/pages/mass_account.php @@ -16,6 +16,8 @@ $title = 'Mass Account Actions'; +csrfProtect(); + $hasCoinsColumn = $db->hasColumn('accounts', 'coins'); $hasPointsColumn = $db->hasColumn('accounts', 'premium_points'); $freePremium = $config['lua']['freePremium']; diff --git a/admin/pages/mass_teleport.php b/admin/pages/mass_teleport.php index 5027fa1cd6..f2a7ee27e7 100644 --- a/admin/pages/mass_teleport.php +++ b/admin/pages/mass_teleport.php @@ -16,6 +16,8 @@ $title = 'Mass Teleport Actions'; +csrfProtect(); + function admin_teleport_position($x, $y, $z) { if (!Player::query()->update([ 'posx' => $x, 'posy' => $y, 'posz' => $z diff --git a/admin/pages/menus.php b/admin/pages/menus.php index a0b492dfe4..4a908eb513 100644 --- a/admin/pages/menus.php +++ b/admin/pages/menus.php @@ -13,19 +13,21 @@ defined('MYAAC') or die('Direct access not allowed!'); $title = 'Menus'; +csrfProtect(); + if (!hasFlag(FLAG_CONTENT_MENUS) && !superAdmin()) { echo 'Access denied.'; return; } -if (isset($_REQUEST['template'])) { - $template = $_REQUEST['template']; +if (isset($_POST['template'])) { + $template = $_POST['template']; - if (isset($_REQUEST['menu'])) { - $post_menu = $_REQUEST['menu']; - $post_menu_link = $_REQUEST['menu_link']; - $post_menu_blank = $_REQUEST['menu_blank']; - $post_menu_color = $_REQUEST['menu_color']; + if (isset($_POST['menu'])) { + $post_menu = $_POST['menu']; + $post_menu_link = $_POST['menu_link']; + $post_menu_blank = $_POST['menu_blank']; + $post_menu_color = $_POST['menu_color']; if (count($post_menu) != count($post_menu_link)) { echo 'Menu count is not equal menu links. Something went wrong when sending form.'; return; @@ -69,9 +71,10 @@ return; } - if (isset($_REQUEST['reset_colors'])) { + if (isset($_GET['reset_colors'])) { if (isset($config['menu_default_color'])) { Menu::where('template', $template)->update(['color' => str_replace('#', '', $config['menu_default_color'])]); + success('Colors has been reset.'); } else { warning('There is no default color defined, cannot reset colors.'); @@ -93,6 +96,7 @@

+ @@ -112,6 +116,7 @@ $last_id = array(); ?>