diff --git a/inc/config.php b/inc/config.php index cd5896fc..3b449b5e 100644 --- a/inc/config.php +++ b/inc/config.php @@ -686,6 +686,7 @@ $config['error']['404'] = _('Page not found.'); $config['error']['modexists'] = _('That mod already exists!'); $config['error']['invalidtheme'] = _('That theme doesn\'t exist!'); + $config['error']['csrf'] = _('Invalid security token! Please go back and try again.'); /* * ========================= @@ -754,9 +755,6 @@ * Mod settings * ==================== */ - - // Server-side confirm button for actions like deleting posts, for when Javascript is disabled or the DOM isn't loaded. - $config['mod']['server-side_confirm'] = true; // Whether or not to lock moderator sessions to the IP address that was logged in with. $config['mod']['lock_ip'] = true; diff --git a/inc/display.php b/inc/display.php index 538d5b03..1be066c2 100644 --- a/inc/display.php +++ b/inc/display.php @@ -86,11 +86,11 @@ function error($message, $priority = true) { function loginForm($error=false, $username=false, $redirect=false) { global $config; - die(Element('page.html', Array( - 'index'=>$config['root'], - 'title'=>_('Login'), - 'config'=>$config, - 'body'=>Element('login.html', Array( + die(Element('page.html', array( + 'index' => $config['root'], + 'title' => _('Login'), + 'config' => $config, + 'body' => Element('login.html', array( 'config'=>$config, 'error'=>$error, 'username'=>utf8tohtml($username), @@ -205,12 +205,13 @@ function truncate($body, $url, $max_lines = false, $max_chars = false) { return $body; } -function confirmLink($text, $title, $confirm, $href) { - global $config, $mod; - if ($config['mod']['server-side_confirm']) - return '' . $text . ''; - else - return '' . $text . ''; +function secure_link_confirm($text, $title, $confirm_message, $href) { + global $config; + + return '' . $text . ''; +} +function secure_link($href) { + return $href . '/' . make_secure_link_token($href); } class Post { @@ -264,15 +265,15 @@ class Post { // Delete if (hasPermission($config['mod']['delete'], $board['uri'], $this->mod)) - $built .= ' ' . confirmLink($config['mod']['link_delete'], 'Delete', 'Are you sure you want to delete this?', $board['uri'] . '/delete/' . $this->id); + $built .= ' ' . secure_link_confirm($config['mod']['link_delete'], 'Delete', 'Are you sure you want to delete this?', $board['uri'] . '/delete/' . $this->id); // Delete all posts by IP if (hasPermission($config['mod']['deletebyip'], $board['uri'], $this->mod)) - $built .= ' ' . confirmLink($config['mod']['link_deletebyip'], 'Delete all posts by IP', 'Are you sure you want to delete all posts by this IP address?', $board['uri'] . '/deletebyip/' . $this->id); + $built .= ' ' . secure_link_confirm($config['mod']['link_deletebyip'], 'Delete all posts by IP', 'Are you sure you want to delete all posts by this IP address?', $board['uri'] . '/deletebyip/' . $this->id); // Delete all posts by IP (global) if (hasPermission($config['mod']['deletebyip_global'], $board['uri'], $this->mod)) - $built .= ' ' . confirmLink($config['mod']['link_deletebyip_global'], 'Delete all posts by IP across all boards', 'Are you sure you want to delete all posts by this IP address, across all boards?', $board['uri'] . '/deletebyip/' . $this->id . '/global'); + $built .= ' ' . secure_link_confirm($config['mod']['link_deletebyip_global'], 'Delete all posts by IP across all boards', 'Are you sure you want to delete all posts by this IP address, across all boards?', $board['uri'] . '/deletebyip/' . $this->id . '/global'); // Ban if (hasPermission($config['mod']['ban'], $board['uri'], $this->mod)) @@ -362,15 +363,15 @@ class Thread { // Mod controls (on posts) // Delete if (hasPermission($config['mod']['delete'], $board['uri'], $this->mod)) - $built .= ' ' . confirmLink($config['mod']['link_delete'], 'Delete', 'Are you sure you want to delete this?', $board['uri'] . '/delete/' . $this->id); + $built .= ' ' . secure_link_confirm($config['mod']['link_delete'], 'Delete', 'Are you sure you want to delete this?', $board['uri'] . '/delete/' . $this->id); // Delete all posts by IP if (hasPermission($config['mod']['deletebyip'], $board['uri'], $this->mod)) - $built .= ' ' . confirmLink($config['mod']['link_deletebyip'], 'Delete all posts by IP', 'Are you sure you want to delete all posts by this IP address?', $board['uri'] . '/deletebyip/' . $this->id); + $built .= ' ' . secure_link_confirm($config['mod']['link_deletebyip'], 'Delete all posts by IP', 'Are you sure you want to delete all posts by this IP address?', $board['uri'] . '/deletebyip/' . $this->id); // Delete all posts by IP (global) if (hasPermission($config['mod']['deletebyip_global'], $board['uri'], $this->mod)) - $built .= ' ' . confirmLink($config['mod']['link_deletebyip_global'], 'Delete all posts by IP across all boards', 'Are you sure you want to delete all posts by this IP address, across all boards?', $board['uri'] . '/deletebyip/' . $this->id . '/global'); + $built .= ' ' . secure_link_confirm($config['mod']['link_deletebyip_global'], 'Delete all posts by IP across all boards', 'Are you sure you want to delete all posts by this IP address, across all boards?', $board['uri'] . '/deletebyip/' . $this->id . '/global'); // Ban if (hasPermission($config['mod']['ban'], $board['uri'], $this->mod)) @@ -393,16 +394,16 @@ class Thread { if (hasPermission($config['mod']['bumplock'], $board['uri'], $this->mod)) if ($this->bumplocked) - $built .= ' ' . $config['mod']['link_bumpunlock'] . ''; + $built .= ' ' . $config['mod']['link_bumpunlock'] . ''; else - $built .= ' ' . $config['mod']['link_bumplock'] . ''; + $built .= ' ' . $config['mod']['link_bumplock'] . ''; // Lock if (hasPermission($config['mod']['lock'], $board['uri'], $this->mod)) if ($this->locked) - $built .= ' ' . $config['mod']['link_unlock'] . ''; + $built .= ' ' . $config['mod']['link_unlock'] . ''; else - $built .= ' ' . $config['mod']['link_lock'] . ''; + $built .= ' ' . $config['mod']['link_lock'] . ''; if (hasPermission($config['mod']['move'], $board['uri'], $this->mod)) $built .= ' ' . $config['mod']['link_move'] . ''; diff --git a/inc/mod/auth.php b/inc/mod/auth.php index d200c23f..6e2d3d1e 100644 --- a/inc/mod/auth.php +++ b/inc/mod/auth.php @@ -150,3 +150,9 @@ function create_pm_header() { return $header; } +function make_secure_link_token($uri) { + global $mod, $config; + return substr(sha1($config['cookies']['salt'] . '-' . $uri . '-' . $mod['id']), 0, 8); +} + + diff --git a/inc/mod/pages.php b/inc/mod/pages.php index ad69ae39..19ff8806 100644 --- a/inc/mod/pages.php +++ b/inc/mod/pages.php @@ -60,7 +60,7 @@ function mod_login() { } function mod_confirm($request) { - mod_page(_('Confirm action'), 'mod/confirm.html', array('request' => $request)); + mod_page(_('Confirm action'), 'mod/confirm.html', array('request' => $request, 'token' => make_secure_link_token($request))); } function mod_logout() { @@ -563,7 +563,7 @@ function mod_ban() { error($config['error']['noaccess']); if (!isset($_POST['ip'], $_POST['reason'], $_POST['length'], $_POST['board'])) { - mod_page(_('New ban'), 'mod/ban_form.html', array()); + mod_page(_('New ban'), 'mod/ban_form.html', array('token' => make_secure_link_token('ban'))); return; } @@ -883,10 +883,12 @@ function mod_move($originBoard, $postID) { if (count($boards) <= 1) error(_('Impossible to move thread; there is only one board.')); - mod_page(_('Move thread'), 'mod/move.html', array('post' => $postID, 'board' => $originBoard, 'boards' => $boards)); + $security_token = make_secure_link_token($originBoard . '/move/' . $postID); + + mod_page(_('Move thread'), 'mod/move.html', array('post' => $postID, 'board' => $originBoard, 'boards' => $boards, 'token' => $security_token)); } -function mod_ban_post($board, $delete, $post) { +function mod_ban_post($board, $delete, $post, $token = false) { global $config, $mod; if (!openBoard($board)) @@ -895,6 +897,8 @@ function mod_ban_post($board, $delete, $post) { if (!hasPermission($config['mod']['delete'], $board)) error($config['error']['noaccess']); + $security_token = make_secure_link_token($board . '/ban' . ($delete ? '&delete' : '') . '/' . $post); + $query = prepare(sprintf('SELECT `ip`, `thread` FROM `posts_%s` WHERE `id` = :id', $board)); $query->bindValue(':id', $post); $query->execute() or error(db_error($query)); @@ -903,7 +907,7 @@ function mod_ban_post($board, $delete, $post) { $thread = $_post['thread']; $ip = $_post['ip']; - + if (isset($_POST['new_ban'], $_POST['reason'], $_POST['length'], $_POST['board'])) { require_once 'inc/mod/ban.php'; @@ -939,7 +943,8 @@ function mod_ban_post($board, $delete, $post) { 'post' => $post, 'board' => $board, 'delete' => (bool)$delete, - 'boards' => listBoards() + 'boards' => listBoards(), + 'token' => $security_token ); mod_page(_('New ban'), 'mod/ban_form.html', $args); diff --git a/mod.php b/mod.php index 52f340d2..20073a3d 100644 --- a/mod.php +++ b/mod.php @@ -50,20 +50,21 @@ $pages = array( '/reports' => 'reports', // report queue '/reports/(\d+)/dismiss(all)?' => 'report_dismiss', // dismiss a report - '/ban' => 'ban', // new ban '/IP/([\w.:]+)' => 'ip', // view ip address '/IP/([\w.:]+)/remove_note/(\d+)' => 'ip_remove_note', // remove note from ip address '/bans' => 'bans', // ban list '/bans/(\d+)' => 'bans', // ban list - - '/(\w+)/delete/(\d+)' => 'delete', // delete post - '/(\w+)/ban(&delete)?/(\d+)' => 'ban_post', // ban poster - '/(\w+)/deletefile/(\d+)' => 'deletefile', // delete file from post - '/(\w+)/deletebyip/(\d+)(/global)?' => 'deletebyip', // delete all posts by IP address - '/(\w+)/(un)?lock/(\d+)' => 'lock', // lock thread - '/(\w+)/(un)?sticky/(\d+)' => 'sticky', // sticky thread - '/(\w+)/bump(un)?lock/(\d+)' => 'bumplock', // "bumplock" thread - '/(\w+)/move/(\d+)' => 'move', // move thread + + // CSRF-protected moderator actions + '/ban' => 'secure_POST ban', // new ban + '/(\w+)/ban(&delete)?/(\d+)' => 'secure_POST ban_post', // ban poster + '/(\w+)/move/(\d+)' => 'secure_POST move', // move thread + '/(\w+)/delete/(\d+)' => 'secure delete', // delete post + '/(\w+)/deletefile/(\d+)' => 'secure deletefile', // delete file from post + '/(\w+)/deletebyip/(\d+)(/global)?' => 'secure deletebyip', // delete all posts by IP address + '/(\w+)/(un)?lock/(\d+)' => 'secure lock', // lock thread + '/(\w+)/(un)?sticky/(\d+)' => 'secure sticky', // sticky thread + '/(\w+)/bump(un)?lock/(\d+)' => 'secure bumplock', // "bumplock" thread '/themes' => 'themes_list', // manage themes '/themes/(\w+)' => 'theme_configure', // configure/reconfigure theme @@ -98,6 +99,8 @@ if (isset($config['mod']['custom_pages'])) { $new_pages = array(); foreach ($pages as $key => $callback) { + if (preg_match('/^secure /', $callback)) + $key .= '(/(?P[a-f0-9]{8}))?'; $new_pages[@$key[0] == '!' ? $key : "!^$key$!"] = $callback; } $pages = $new_pages; @@ -106,6 +109,29 @@ foreach ($pages as $uri => $handler) { if (preg_match($uri, $query, $matches)) { $matches = array_slice($matches, 1); + if (preg_match('/^secure(_POST)? /', $handler, $m)) { + $secure_post_only = isset($m[1]); + if (!$secure_post_only || $_SERVER['REQUEST_METHOD'] == 'POST') { + $token = isset($matches['token']) ? $matches['token'] : (isset($_POST['token']) ? $_POST['token'] : false); + + if ($token === false) { + if ($secure_post_only) + error($config['error']['csrf']); + else { + mod_confirm(substr($query, 1)); + exit; + } + } + + // CSRF-protected page; validate security token + $actual_query = preg_replace('!/([a-f0-9]{8})$!', '', $query); + if ($token != make_secure_link_token(substr($actual_query, 1))) { + error($config['error']['csrf']); + } + } + $handler = preg_replace('/^secure(_POST)? /', '', $handler); + } + if ($config['debug']) { $debug['mod_page'] = array( 'req' => $query, diff --git a/templates/mod/ban_form.html b/templates/mod/ban_form.html index 5d5f2b81..edc1ab3b 100644 --- a/templates/mod/ban_form.html +++ b/templates/mod/ban_form.html @@ -5,6 +5,7 @@ {% endif %}
+ {% if redirect %} {% endif %} diff --git a/templates/mod/confirm.html b/templates/mod/confirm.html index 6dc4d66a..65e43eb3 100644 --- a/templates/mod/confirm.html +++ b/templates/mod/confirm.html @@ -1,7 +1,7 @@

- {% trans 'Are you sure you want to do that?' %} {% trans 'Click to proceed to' %} ?/{{ request }}. + {% trans 'Are you sure you want to do that?' %} {% trans 'Click to proceed to' %} ?/{{ request }}.

- {% trans 'You are seeing this message because we were unable to serve a confirmation dialog, probably due to Javascript being disabled.' %} + {% trans 'You are probably seeing this message because Javascript being disabled. This is a necessary security measure to prevent CSRF attacks.' %}

diff --git a/templates/mod/move.html b/templates/mod/move.html index 802903b2..59bd60f6 100644 --- a/templates/mod/move.html +++ b/templates/mod/move.html @@ -1,4 +1,5 @@ - + +
@@ -23,7 +24,7 @@
    {% for targetboard in boards if targetboard.uri != board %}
  • - +