CSRF protection

This commit is contained in:
Michael Save 2012-08-27 15:19:05 +10:00
parent 4a9d497a94
commit 6229b82a43
8 changed files with 82 additions and 44 deletions

View File

@ -686,6 +686,7 @@
$config['error']['404'] = _('Page not found.'); $config['error']['404'] = _('Page not found.');
$config['error']['modexists'] = _('That mod <a href="?/users/%d">already exists</a>!'); $config['error']['modexists'] = _('That mod <a href="?/users/%d">already exists</a>!');
$config['error']['invalidtheme'] = _('That theme doesn\'t exist!'); $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 * 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. // Whether or not to lock moderator sessions to the IP address that was logged in with.
$config['mod']['lock_ip'] = true; $config['mod']['lock_ip'] = true;

View File

@ -86,11 +86,11 @@ function error($message, $priority = true) {
function loginForm($error=false, $username=false, $redirect=false) { function loginForm($error=false, $username=false, $redirect=false) {
global $config; global $config;
die(Element('page.html', Array( die(Element('page.html', array(
'index'=>$config['root'], 'index' => $config['root'],
'title'=>_('Login'), 'title' => _('Login'),
'config'=>$config, 'config' => $config,
'body'=>Element('login.html', Array( 'body' => Element('login.html', array(
'config'=>$config, 'config'=>$config,
'error'=>$error, 'error'=>$error,
'username'=>utf8tohtml($username), 'username'=>utf8tohtml($username),
@ -205,12 +205,13 @@ function truncate($body, $url, $max_lines = false, $max_chars = false) {
return $body; return $body;
} }
function confirmLink($text, $title, $confirm, $href) { function secure_link_confirm($text, $title, $confirm_message, $href) {
global $config, $mod; global $config;
if ($config['mod']['server-side_confirm'])
return '<a onclick="if (confirm(\'' . htmlentities(addslashes($confirm)) . '\')) document.location=\'?/' . htmlentities(addslashes($href)) . '\';return false;" title="' . htmlentities($title) . '" href="?/confirm/' . $href . '">' . $text . '</a>'; return '<a onclick="if (confirm(\'' . htmlentities(addslashes($confirm_message)) . '\')) document.location=\'?/' . htmlentities(addslashes($href . '/' . make_secure_link_token($href))) . '\';return false;" title="' . htmlentities($title) . '" href="?/' . $href . '">' . $text . '</a>';
else }
return '<a onclick="return confirm(\'' . htmlentities(addslashes($confirm)) . '\')" title="' . htmlentities($title) . '" href="?/' . $href . '">' . $text . '</a>'; function secure_link($href) {
return $href . '/' . make_secure_link_token($href);
} }
class Post { class Post {
@ -264,15 +265,15 @@ class Post {
// Delete // Delete
if (hasPermission($config['mod']['delete'], $board['uri'], $this->mod)) 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 // Delete all posts by IP
if (hasPermission($config['mod']['deletebyip'], $board['uri'], $this->mod)) 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) // Delete all posts by IP (global)
if (hasPermission($config['mod']['deletebyip_global'], $board['uri'], $this->mod)) 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 // Ban
if (hasPermission($config['mod']['ban'], $board['uri'], $this->mod)) if (hasPermission($config['mod']['ban'], $board['uri'], $this->mod))
@ -362,15 +363,15 @@ class Thread {
// Mod controls (on posts) // Mod controls (on posts)
// Delete // Delete
if (hasPermission($config['mod']['delete'], $board['uri'], $this->mod)) 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 // Delete all posts by IP
if (hasPermission($config['mod']['deletebyip'], $board['uri'], $this->mod)) 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) // Delete all posts by IP (global)
if (hasPermission($config['mod']['deletebyip_global'], $board['uri'], $this->mod)) 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 // Ban
if (hasPermission($config['mod']['ban'], $board['uri'], $this->mod)) 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 (hasPermission($config['mod']['bumplock'], $board['uri'], $this->mod))
if ($this->bumplocked) if ($this->bumplocked)
$built .= ' <a title="Allow thread to be bumped" href="?/' . $board['uri'] . '/bumpunlock/' . $this->id . '">' . $config['mod']['link_bumpunlock'] . '</a>'; $built .= ' <a title="Allow thread to be bumped" href="?/' . secure_link($board['uri'] . '/bumpunlock/' . $this->id) . '">' . $config['mod']['link_bumpunlock'] . '</a>';
else else
$built .= ' <a title="Prevent thread from being bumped" href="?/' . $board['uri'] . '/bumplock/' . $this->id . '">' . $config['mod']['link_bumplock'] . '</a>'; $built .= ' <a title="Prevent thread from being bumped" href="?/' . secure_link($board['uri'] . '/bumplock/' . $this->id) . '">' . $config['mod']['link_bumplock'] . '</a>';
// Lock // Lock
if (hasPermission($config['mod']['lock'], $board['uri'], $this->mod)) if (hasPermission($config['mod']['lock'], $board['uri'], $this->mod))
if ($this->locked) if ($this->locked)
$built .= ' <a title="Unlock thread" href="?/' . $board['uri'] . '/unlock/' . $this->id . '">' . $config['mod']['link_unlock'] . '</a>'; $built .= ' <a title="Unlock thread" href="?/' . secure_link($board['uri'] . '/unlock/' . $this->id) . '">' . $config['mod']['link_unlock'] . '</a>';
else else
$built .= ' <a title="Lock thread" href="?/' . $board['uri'] . '/lock/' . $this->id . '">' . $config['mod']['link_lock'] . '</a>'; $built .= ' <a title="Lock thread" href="?/' . secure_link($board['uri'] . '/lock/' . $this->id) . '">' . $config['mod']['link_lock'] . '</a>';
if (hasPermission($config['mod']['move'], $board['uri'], $this->mod)) if (hasPermission($config['mod']['move'], $board['uri'], $this->mod))
$built .= ' <a title="Move thread to another board" href="?/' . $board['uri'] . '/move/' . $this->id . '">' . $config['mod']['link_move'] . '</a>'; $built .= ' <a title="Move thread to another board" href="?/' . $board['uri'] . '/move/' . $this->id . '">' . $config['mod']['link_move'] . '</a>';

View File

@ -150,3 +150,9 @@ function create_pm_header() {
return $header; return $header;
} }
function make_secure_link_token($uri) {
global $mod, $config;
return substr(sha1($config['cookies']['salt'] . '-' . $uri . '-' . $mod['id']), 0, 8);
}

View File

@ -60,7 +60,7 @@ function mod_login() {
} }
function mod_confirm($request) { 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() { function mod_logout() {
@ -563,7 +563,7 @@ function mod_ban() {
error($config['error']['noaccess']); error($config['error']['noaccess']);
if (!isset($_POST['ip'], $_POST['reason'], $_POST['length'], $_POST['board'])) { 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; return;
} }
@ -883,10 +883,12 @@ function mod_move($originBoard, $postID) {
if (count($boards) <= 1) if (count($boards) <= 1)
error(_('Impossible to move thread; there is only one board.')); 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; global $config, $mod;
if (!openBoard($board)) if (!openBoard($board))
@ -895,6 +897,8 @@ function mod_ban_post($board, $delete, $post) {
if (!hasPermission($config['mod']['delete'], $board)) if (!hasPermission($config['mod']['delete'], $board))
error($config['error']['noaccess']); 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 = prepare(sprintf('SELECT `ip`, `thread` FROM `posts_%s` WHERE `id` = :id', $board));
$query->bindValue(':id', $post); $query->bindValue(':id', $post);
$query->execute() or error(db_error($query)); $query->execute() or error(db_error($query));
@ -903,7 +907,7 @@ function mod_ban_post($board, $delete, $post) {
$thread = $_post['thread']; $thread = $_post['thread'];
$ip = $_post['ip']; $ip = $_post['ip'];
if (isset($_POST['new_ban'], $_POST['reason'], $_POST['length'], $_POST['board'])) { if (isset($_POST['new_ban'], $_POST['reason'], $_POST['length'], $_POST['board'])) {
require_once 'inc/mod/ban.php'; require_once 'inc/mod/ban.php';
@ -939,7 +943,8 @@ function mod_ban_post($board, $delete, $post) {
'post' => $post, 'post' => $post,
'board' => $board, 'board' => $board,
'delete' => (bool)$delete, 'delete' => (bool)$delete,
'boards' => listBoards() 'boards' => listBoards(),
'token' => $security_token
); );
mod_page(_('New ban'), 'mod/ban_form.html', $args); mod_page(_('New ban'), 'mod/ban_form.html', $args);

46
mod.php
View File

@ -50,20 +50,21 @@ $pages = array(
'/reports' => 'reports', // report queue '/reports' => 'reports', // report queue
'/reports/(\d+)/dismiss(all)?' => 'report_dismiss', // dismiss a report '/reports/(\d+)/dismiss(all)?' => 'report_dismiss', // dismiss a report
'/ban' => 'ban', // new ban
'/IP/([\w.:]+)' => 'ip', // view ip address '/IP/([\w.:]+)' => 'ip', // view ip address
'/IP/([\w.:]+)/remove_note/(\d+)' => 'ip_remove_note', // remove note from ip address '/IP/([\w.:]+)/remove_note/(\d+)' => 'ip_remove_note', // remove note from ip address
'/bans' => 'bans', // ban list '/bans' => 'bans', // ban list
'/bans/(\d+)' => 'bans', // ban list '/bans/(\d+)' => 'bans', // ban list
'/(\w+)/delete/(\d+)' => 'delete', // delete post // CSRF-protected moderator actions
'/(\w+)/ban(&delete)?/(\d+)' => 'ban_post', // ban poster '/ban' => 'secure_POST ban', // new ban
'/(\w+)/deletefile/(\d+)' => 'deletefile', // delete file from post '/(\w+)/ban(&delete)?/(\d+)' => 'secure_POST ban_post', // ban poster
'/(\w+)/deletebyip/(\d+)(/global)?' => 'deletebyip', // delete all posts by IP address '/(\w+)/move/(\d+)' => 'secure_POST move', // move thread
'/(\w+)/(un)?lock/(\d+)' => 'lock', // lock thread '/(\w+)/delete/(\d+)' => 'secure delete', // delete post
'/(\w+)/(un)?sticky/(\d+)' => 'sticky', // sticky thread '/(\w+)/deletefile/(\d+)' => 'secure deletefile', // delete file from post
'/(\w+)/bump(un)?lock/(\d+)' => 'bumplock', // "bumplock" thread '/(\w+)/deletebyip/(\d+)(/global)?' => 'secure deletebyip', // delete all posts by IP address
'/(\w+)/move/(\d+)' => 'move', // move thread '/(\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' => 'themes_list', // manage themes
'/themes/(\w+)' => 'theme_configure', // configure/reconfigure theme '/themes/(\w+)' => 'theme_configure', // configure/reconfigure theme
@ -98,6 +99,8 @@ if (isset($config['mod']['custom_pages'])) {
$new_pages = array(); $new_pages = array();
foreach ($pages as $key => $callback) { foreach ($pages as $key => $callback) {
if (preg_match('/^secure /', $callback))
$key .= '(/(?P<token>[a-f0-9]{8}))?';
$new_pages[@$key[0] == '!' ? $key : "!^$key$!"] = $callback; $new_pages[@$key[0] == '!' ? $key : "!^$key$!"] = $callback;
} }
$pages = $new_pages; $pages = $new_pages;
@ -106,6 +109,29 @@ foreach ($pages as $uri => $handler) {
if (preg_match($uri, $query, $matches)) { if (preg_match($uri, $query, $matches)) {
$matches = array_slice($matches, 1); $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']) { if ($config['debug']) {
$debug['mod_page'] = array( $debug['mod_page'] = array(
'req' => $query, 'req' => $query,

View File

@ -5,6 +5,7 @@
{% endif %} {% endif %}
<form action="{{ action }}" method="post"> <form action="{{ action }}" method="post">
<input type="hidden" name="token" value="{{ token }}">
{% if redirect %} {% if redirect %}
<input type="hidden" name="redirect" value="{{ redirect|e }}"> <input type="hidden" name="redirect" value="{{ redirect|e }}">
{% endif %} {% endif %}

View File

@ -1,7 +1,7 @@
<p style="text-align:center;font-size:1.1em"> <p style="text-align:center;font-size:1.1em">
{% trans 'Are you sure you want to do that?' %} <a href="?/{{ request }}">{% trans 'Click to proceed to' %} ?/{{ request }}</a>. {% trans 'Are you sure you want to do that?' %} <a href="?/{{ request }}/{{ token }}">{% trans 'Click to proceed to' %} ?/{{ request }}</a>.
</p> </p>
<p class="unimportant" style="text-align:center"> <p class="unimportant" style="text-align:center">
{% 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.' %}
</p> </p>

View File

@ -1,4 +1,5 @@
<form action="?/{{ board }}/move/{{ post }}" method="post"> <form action="?/{{ board }}/move/{{ post }}" method="post">
<input type="hidden" name="token" value="{{ token }}">
<table> <table>
<tr> <tr>
<th> <th>
@ -23,7 +24,7 @@
<ul style="list-style:none;padding:0"> <ul style="list-style:none;padding:0">
{% for targetboard in boards if targetboard.uri != board %} {% for targetboard in boards if targetboard.uri != board %}
<li> <li>
<input type="radio" name="board" value="{{ targetboard.uri }}" id="ban-board-{{ targetboard.uri }}"> <input type="radio" name="board" value="{{ targetboard.uri }}" id="ban-board-{{ targetboard.uri }}" {% if boards|count == 2 %}checked{% endif %}>
<label style="display:inline" for="ban-board-{{ targetboard.uri }}"> <label style="display:inline" for="ban-board-{{ targetboard.uri }}">
{{ config.board_abbreviation|sprintf(targetboard.uri) }} - {{ targetboard.title|e }} {{ config.board_abbreviation|sprintf(targetboard.uri) }} - {{ targetboard.title|e }}
</label> </label>