[opw-web] Guard against cross site request forgery (CSRF)
- From: Owen Taylor <otaylor src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [opw-web] Guard against cross site request forgery (CSRF)
- Date: Tue, 11 Mar 2014 22:17:35 +0000 (UTC)
commit 50008a1b98ae199e799d1aff46688e15e91a9680
Author: Owen W. Taylor <otaylor fishsoup net>
Date: Tue Mar 11 18:05:13 2014 -0400
Guard against cross site request forgery (CSRF)
* Add a hidden field to the main form with a token generated as the
HMAC of the session ID with a secret key.
* For all cases where we modify the database, check for the presence
of the token.
* Switch cases where we were using GET links for side effects to post
the main form via Javascript (could also add the token to GET urls,
but avoiding GET with side effects is a small plus.)
classes/class_config.php | 4 ++
classes/class_core.php | 2 +
classes/class_skin.php | 1 +
classes/class_user.php | 42 ++++++++++++++++++++
config.sample.php | 3 +
lang/en-gb.php | 3 +
modules/mod_approve_mentors.php | 2 +
modules/mod_attachment.php | 4 ++
modules/mod_manage_organizations.php | 4 ++
modules/mod_manage_programs.php | 4 ++
modules/mod_manage_projects.php | 2 +
modules/mod_notifications.php | 2 +
modules/mod_user_ban.php | 4 ++
modules/mod_user_profile.php | 2 +
modules/mod_view_projects.php | 16 +++++++
.../html/tpl_approve_mentors_item.html | 4 +-
skins/easterngreen/html/tpl_header.html | 1 +
.../html/tpl_manage_projects_item.html | 4 +-
skins/easterngreen/html/tpl_program_home.html | 6 ++-
skins/easterngreen/html/tpl_user_ban_item.html | 4 +-
.../easterngreen/html/tpl_view_projects_item.html | 4 +-
skins/easterngreen/js/main.js | 13 ++++++
skins/neverland/html/tpl_approve_mentors_item.html | 6 +-
skins/neverland/html/tpl_header.html | 1 +
skins/neverland/html/tpl_program_home.html | 6 ++-
skins/neverland/html/tpl_user_ban_item.html | 4 +-
skins/neverland/html/tpl_view_projects_item.html | 6 +-
skins/neverland/js/main.js | 13 ++++++
28 files changed, 147 insertions(+), 20 deletions(-)
---
diff --git a/classes/class_config.php b/classes/class_config.php
index c4ebb0d..93f73af 100644
--- a/classes/class_config.php
+++ b/classes/class_config.php
@@ -15,6 +15,8 @@ class config
var $db_password;
var $db_prefix;
+ var $server_secret;
+
var $enable_cache;
var $auth_openid_enabled;
@@ -99,6 +101,8 @@ class config
$this->db_password = isset($db_password) ? $db_password : '';
$this->db_prefix = isset($db_prefix) ? $db_prefix : '';
+ $this->server_secret = isset($server_secret) ? $server_secret : '';
+
$this->enable_cache = isset($enable_cache) ? $enable_cache : '';
$this->auth_openid_enabled = isset($auth_openid_enabled) ? $auth_openid_enabled : false;
diff --git a/classes/class_core.php b/classes/class_core.php
index 28cdb4d..07df629 100644
--- a/classes/class_core.php
+++ b/classes/class_core.php
@@ -13,6 +13,8 @@ class core
// Constructor
function __construct()
{
+ global $config;
+
$this->timestamp = time();
}
diff --git a/classes/class_skin.php b/classes/class_skin.php
index 823f8c4..733fdee 100644
--- a/classes/class_skin.php
+++ b/classes/class_skin.php
@@ -171,6 +171,7 @@ class skin
'moderator_visibility' => $this->visibility($user->is_admin),
'admin_visibility' => $this->visibility($user->is_admin),
'nav_home' => $core->path(),
+ 'csrf_token' => $user->csrf_token()
);
if ($program_id > 0)
diff --git a/classes/class_user.php b/classes/class_user.php
index 3216262..1af04e9 100644
--- a/classes/class_user.php
+++ b/classes/class_user.php
@@ -570,4 +570,46 @@ class user
{
return str_replace(array('*', '\\', '(', ')'), array('\\*', '\\\\', '\\(', '\\)'), $string);
}
+
+ function csrf_token()
+ {
+ global $config;
+
+ if (!$this->sid)
+ return '';
+
+ // The advantage of using this rather then just the session ID itself
+ // is that if the token leaks (by an URL, say) an attacker won't be
+ // able to use it without also getting the session ID.
+ // Note that HMAC-MD5 is still considered very strong desite
+ // attachs on MD5 - but no downside from using a better algorithm.
+ return hash_hmac('sha256', $this->sid, $config->server_secret);
+ }
+
+ function check_csrf()
+ {
+ global $core, $lang;
+
+ // The most likely reason for a CSRF prevention validation failure is
+ // that the user logged out from a different browser tab, and either
+ // is still logged out, or logged in again. The second most likely
+ // reason is a site bug.
+
+ // We require POST here - it would be possible to embed the CSRF token
+ // in the URL for a GET, but GET requests shouldn't have side effects
+ // in any case.
+
+ if (!$this->sid ||
+ $this->csrf_token() != $_POST['tok'])
+ {
+ header("HTTP/1.0 403 Forbidden");
+
+ if (!$this->sid)
+ echo $lang->get("csrf_not_logged_in");
+ else
+ echo $lang->get("csrf_mismatch");
+
+ exit;
+ }
+ }
}
diff --git a/config.sample.php b/config.sample.php
index cf3c599..8ae44ba 100644
--- a/config.sample.php
+++ b/config.sample.php
@@ -28,6 +28,9 @@ $db_password = "";
// Table prefix
$db_prefix = "";
+// Server secret key, used for CSRF protection, generate with: openssl rand -base64 18
+$server_secret = '';
+
// Whether caching is turned on if Cache-Lite is installed; turn off for development
$enable_cache = true;
diff --git a/lang/en-gb.php b/lang/en-gb.php
index 38ccaac..1cd8dea 100644
--- a/lang/en-gb.php
+++ b/lang/en-gb.php
@@ -15,6 +15,9 @@ $lang_data = array(
'site_title' => "Outreach Program For Women - Free and Open Source Software internships for
women (cis and trans) and genderqueer",
'site_desc' => "The Outreach Program for Women (OPW) helps women (cis and trans) and
genderqueer get involved in free and open source software. We provide a supportive community for beginning to
contribute any time throughout the year and offer focused internship opportunities twice a year with a number
of free software organizations.",
+ 'csrf_not_logged_in' => 'Please hit back, log in, and try again.',
+ 'csrf_mismatch' => 'Please hit back, reload the page, and try again.',
+
/* Global keys */
'kde_links' => 'KDE Links',
'not_logged_in' => 'You are not logged in',
diff --git a/modules/mod_approve_mentors.php b/modules/mod_approve_mentors.php
index 2c12557..e7721f6 100644
--- a/modules/mod_approve_mentors.php
+++ b/modules/mod_approve_mentors.php
@@ -13,6 +13,8 @@ $username = $core->variable('u', '');
// Something was modified
if (!empty($action) && !empty($username))
{
+ $user->check_csrf();
+
// Process the username
$username = urldecode($username);
diff --git a/modules/mod_attachment.php b/modules/mod_attachment.php
index 2e3d350..7d1783c 100644
--- a/modules/mod_attachment.php
+++ b/modules/mod_attachment.php
@@ -62,6 +62,8 @@ if ($action == 'add') {
$error_message = '';
if ($attachment_add) {
+ $user->check_csrf();
+
if ($error_message === '') {
if ($description == '') {
$error_message = $lang->get('upload_description_needed');
@@ -140,6 +142,8 @@ if ($action == 'add') {
// Deletion was confirmed
if ($confirm)
{
+ $user->check_csrf();
+
$sql = "DELETE FROM {$db->prefix}attachments " .
"WHERE id = ?";
$db->query($sql, $attachment_id);
diff --git a/modules/mod_manage_organizations.php b/modules/mod_manage_organizations.php
index 7028974..03a07c3 100644
--- a/modules/mod_manage_organizations.php
+++ b/modules/mod_manage_organizations.php
@@ -75,6 +75,8 @@ else if ($action == 'editor')
if ($organization_save)
{
+ $user->check_csrf();
+
if (empty($title))
{
$error_message = $lang->get('err_mandatory_fields');
@@ -144,6 +146,8 @@ else if ($action == 'delete')
// Deletion was confirmed
if ($confirm)
{
+ $user->check_csrf();
+
$params = array('id' => $id);
// XXX FIXME
diff --git a/modules/mod_manage_programs.php b/modules/mod_manage_programs.php
index d00054a..78b61b2 100644
--- a/modules/mod_manage_programs.php
+++ b/modules/mod_manage_programs.php
@@ -79,6 +79,8 @@ else if ($action == 'editor')
if ($program_save)
{
+ $user->check_csrf();
+
if (empty($title) || empty($start_date) || empty($end_date) ||
empty($dl_student_date) || empty($dl_mentor_date))
{
@@ -203,6 +205,8 @@ else if ($action == 'delete')
// Deletion was confirmed
if ($confirm)
{
+ $user->check_csrf();
+
$params = array('id' => $id);
$sql = "DELETE FROM {$db->prefix}roles " .
diff --git a/modules/mod_manage_projects.php b/modules/mod_manage_projects.php
index 9005dd4..caddf28 100644
--- a/modules/mod_manage_projects.php
+++ b/modules/mod_manage_projects.php
@@ -66,6 +66,8 @@ if ($organization_id > 0) {
// See if we need to save anything
if ($rankings_save && $can_edit) {
+ $user->check_csrf();
+
foreach ($list_data as &$row) {
$project_id = $row['project_id'];
$ranking_field_name = 'ranking' . $project_id;
diff --git a/modules/mod_notifications.php b/modules/mod_notifications.php
index dd239d2..884385b 100644
--- a/modules/mod_notifications.php
+++ b/modules/mod_notifications.php
@@ -20,6 +20,8 @@ $output = '';
if (isset($_POST['process']))
{
+ $user->check_csrf();
+
// Get all entries from the queue and their corresponding program data
$sql = "SELECT prg.id as program_id, " .
" prg.title as program_title, " .
diff --git a/modules/mod_user_ban.php b/modules/mod_user_ban.php
index fd7bed4..4dd2baf 100644
--- a/modules/mod_user_ban.php
+++ b/modules/mod_user_ban.php
@@ -19,6 +19,8 @@ $is_successful = $success == 1;
// Was the ban form submitted?
if ($ban_submit)
{
+ $user->check_csrf();
+
$params = array('ban_user' => $ban_user);
// Check if user is already banned
@@ -48,6 +50,8 @@ if ($ban_submit)
// Was the user unbanned
if (!empty($unban_user))
{
+ $user->check_csrf();
+
$params = array('unban_user' => $ban_user);
// Delete the user from the ban table
diff --git a/modules/mod_user_profile.php b/modules/mod_user_profile.php
index 2f0f9e2..110ac0c 100644
--- a/modules/mod_user_profile.php
+++ b/modules/mod_user_profile.php
@@ -108,6 +108,8 @@ if ($action == 'view') {
$user->restrict($can_edit);
if (isset($_POST['user_save'])) {
+ $user->check_csrf();
+
$fullname = $core->variable('fullname', '', false, true);
$email = $core->variable('email', '', false, true);
$website = $core->variable('website', '', false, true);
diff --git a/modules/mod_view_projects.php b/modules/mod_view_projects.php
index 3d013d0..2326c23 100644
--- a/modules/mod_view_projects.php
+++ b/modules/mod_view_projects.php
@@ -160,6 +160,8 @@ if ($action == 'editor')
// Project was saved
if ($project_save)
{
+ $user->check_csrf();
+
// All fields are mandatory
if (empty($title) || empty($description) || $organization_id <= 0)
{
@@ -385,6 +387,8 @@ else if ($action == 'delete')
// Deletion was confirmed
if ($confirm)
{
+ $user->check_csrf();
+
$sql = "DELETE FROM {$db->prefix}participants " .
"WHERE project_id = ?";
$db->query($sql, $project_id);
@@ -511,6 +515,8 @@ else if ($action == 'view')
// User removed themselves as mentor
if ($mentor_remove && $can_remove_mentor)
{
+ $user->check_csrf();
+
$sql = "DELETE FROM {$db->prefix}participants " .
"WHERE username = :username AND project_id = :project_id";
$db->query($sql, array('username' => $user->username,
@@ -540,6 +546,8 @@ else if ($action == 'view')
// User applied as mentor
if ($mentor_apply && $can_mentor)
{
+ $user->check_csrf();
+
$sql = "INSERT INTO {$db->prefix}participants " .
"(username, project_id, program_id, role) " .
"VALUES (:username, :project_id, :program_id, 'm')";
@@ -802,6 +810,8 @@ else if ($action == 'user' || $action == 'proposed' || $action == 'accepted' ||
}
else if ($action == 'approve' || $action == 'reject')
{
+ $user->check_csrf();
+
// This is an admin only action
$user->restrict(false, true);
@@ -838,6 +848,8 @@ else if ($action == 'apply')
// When applying as a mentor, an organization must be selected - otherwise
// we can continue
if ($category == 'student' || ($organization_id > 0 || $organization_id == -1)) {
+ $user->check_csrf();
+
if ($category == 'student')
$organization_id = 0;
@@ -928,6 +940,8 @@ else if ($action == 'resign' || ($role == 'm' || $role == 'i'))
$module_title = null;
$module_data = $skin->output('tpl_error_box');
} else if ($confirm) {
+ $user->check_csrf();
+
$sql = "DELETE FROM {$db->prefix}participants " .
"WHERE username = :username " .
"AND program_id = :program_id";
@@ -971,6 +985,8 @@ else if ($action == 'resign')
if ($confirm)
{
+ $user->check_csrf();
+
// Check if program has already started
$sql = "SELECT COUNT(*) AS count " .
"FROM {$db->prefix}programs " .
diff --git a/skins/easterngreen/html/tpl_approve_mentors_item.html
b/skins/easterngreen/html/tpl_approve_mentors_item.html
index 4c700dd..215e822 100644
--- a/skins/easterngreen/html/tpl_approve_mentors_item.html
+++ b/skins/easterngreen/html/tpl_approve_mentors_item.html
@@ -8,11 +8,11 @@
</td>
<td class="align-right">
- <a href="[[approve_url]]" title="{{approve}}">
+ <a href="#" onclick="submit(event, '[[approve_url]]')" title="{{approve}}">
<i class="icon-ok"></i>
</a>
- <a href="[[reject_url]]" title="{{reject}}">
+ <a href="#" onclick="submit(event, '[[reject_url]]')" title="{{reject}}">
<i class="icon-remove"></i>
</a>
</td>
diff --git a/skins/easterngreen/html/tpl_header.html b/skins/easterngreen/html/tpl_header.html
index 1ef213a..b2b8a29 100644
--- a/skins/easterngreen/html/tpl_header.html
+++ b/skins/easterngreen/html/tpl_header.html
@@ -168,3 +168,4 @@ ocalization variable
<div class="row">
<div class="span12">
<form class="form-horizontal" method="post" enctype="multipart/form-data">
+ <input type="hidden" name="tok" value="[[csrf_token]]"></input>
diff --git a/skins/easterngreen/html/tpl_manage_projects_item.html
b/skins/easterngreen/html/tpl_manage_projects_item.html
index 4c59da1..48e8347 100644
--- a/skins/easterngreen/html/tpl_manage_projects_item.html
+++ b/skins/easterngreen/html/tpl_manage_projects_item.html
@@ -19,11 +19,11 @@
<td>
<div class="align-right [[apprej_visibility]]">
- <a href="[[approve_url]]" title="{{approve}}">
+ <a href="#" onclick="submit(event, '[[approve_url]]')" title="{{approve}}">
<i class="icon-ok"></i>
</a>
- <a href="[[reject_url]]" title="{{reject}}">
+ <a href="#" onclick="submit(event, '[[reject_url]]')" title="{{reject}}">
<i class="icon-remove"></i>
</a>
</div>
diff --git a/skins/easterngreen/html/tpl_program_home.html b/skins/easterngreen/html/tpl_program_home.html
index efbbff0..e5ad767 100644
--- a/skins/easterngreen/html/tpl_program_home.html
+++ b/skins/easterngreen/html/tpl_program_home.html
@@ -60,12 +60,14 @@
<div class="program-actions [[user_visibility]]">
<div class="[[prg_guest_visibility]]">
- <a href="?q=view_projects&prg=[[program_id]]&a=apply&c=student" class="btn btn-large
[[dl_student_visibility]]">
+ <a href="#" onclick="submit(event,
'?q=view_projects&prg=[[program_id]]&a=apply&c=student')"
+ class="btn btn-large [[dl_student_visibility]]">
<img src="[[skin_path]]/images/submit-project.png" width="26" height="26" alt="" />
{{apply_student}}
</a>
- <a href="?q=view_projects&prg=[[program_id]]&a=apply&c=mentor" class="btn btn-large
[[dl_mentor_visibility]]">
+ <a href="#" onclick="submit(event,
'?q=view_projects&prg=[[program_id]]&a=apply&c=mentor')"
+ class="btn btn-large [[dl_mentor_visibility]]">
<img src="[[skin_path]]/images/mentor-project.png" width="26" height="26" alt="" />
{{apply_mentor}}
</a>
diff --git a/skins/easterngreen/html/tpl_user_ban_item.html b/skins/easterngreen/html/tpl_user_ban_item.html
index 53ab396..2734609 100644
--- a/skins/easterngreen/html/tpl_user_ban_item.html
+++ b/skins/easterngreen/html/tpl_user_ban_item.html
@@ -4,8 +4,8 @@
</td>
<td>
- <a href="[[unban_url]]" title="{{unban_user}}">
+ <a href="#" onclick="submit(event, '[[unban_url]]')" title="{{unban_user}}">
<i class="icon-remove"></i>
</a>
</td>
-</tr>
\ No newline at end of file
+</tr>
diff --git a/skins/easterngreen/html/tpl_view_projects_item.html
b/skins/easterngreen/html/tpl_view_projects_item.html
index c9a01e6..f237644 100644
--- a/skins/easterngreen/html/tpl_view_projects_item.html
+++ b/skins/easterngreen/html/tpl_view_projects_item.html
@@ -15,11 +15,11 @@
<td>
<div class="align-right [[apprej_visibility]]">
- <a href="[[approve_url]]" title="{{approve}}">
+ <a href="#" onclick="submit(event, '[[approve_url]]')" title="{{approve}}">
<i class="icon-ok"></i>
</a>
- <a href="[[reject_url]]" title="{{reject}}">
+ <a href="#" onclick="submit(event, '[[reject_url]]')" title="{{reject}}">
<i class="icon-remove"></i>
</a>
</div>
diff --git a/skins/easterngreen/js/main.js b/skins/easterngreen/js/main.js
index c805876..ec05b33 100644
--- a/skins/easterngreen/js/main.js
+++ b/skins/easterngreen/js/main.js
@@ -1,3 +1,16 @@
+function submit(evt, url) {
+ evt.preventDefault();
+ var form = document.getElementsByTagName("form")[0];
+ var oldAction = form.action;
+ form.action = url;
+ try {
+ form.submit();
+ } catch(e) {
+ form.action = oldAction;
+ thow(e);
+ }
+}
+
jQuery(document).ready(function($) {
$(".scroll, .scroll-logo").click(function(event){
event.preventDefault();
diff --git a/skins/neverland/html/tpl_approve_mentors_item.html
b/skins/neverland/html/tpl_approve_mentors_item.html
index 575b7d1..05942ec 100644
--- a/skins/neverland/html/tpl_approve_mentors_item.html
+++ b/skins/neverland/html/tpl_approve_mentors_item.html
@@ -4,12 +4,12 @@
</td>
<td class="align-right">
- <a href="[[approve_url]]" title="{{approve}}">
+ <a href="#" onclick="submit(event, '[[approve_url]]')" title="{{approve}}">
<i class="icon-ok"></i>
</a>
- <a href="[[reject_url]]" title="{{reject}}">
+ <a href="#" onclick="submit(event, '[[reject_url]]')" title="{{reject}}">
<i class="icon-remove"></i>
</a>
</td>
-</tr>
\ No newline at end of file
+</tr>
diff --git a/skins/neverland/html/tpl_header.html b/skins/neverland/html/tpl_header.html
index 4b77198..b031b9d 100644
--- a/skins/neverland/html/tpl_header.html
+++ b/skins/neverland/html/tpl_header.html
@@ -172,3 +172,4 @@ ocalization variable
<div class="row">
<div class="span12">
<form class="form-horizontal" method="post">
+ <input type="hidden" name="tok" value="[[csrf_token]]"></input>
diff --git a/skins/neverland/html/tpl_program_home.html b/skins/neverland/html/tpl_program_home.html
index c5e4adc..a6a1788 100644
--- a/skins/neverland/html/tpl_program_home.html
+++ b/skins/neverland/html/tpl_program_home.html
@@ -67,12 +67,14 @@
<div class="program-actions [[user_visibility]]">
<div class="[[prg_guest_visibility]]">
- <a href="?q=view_projects&prg=[[program_id]]&a=apply&c=student" class="btn btn-large
[[dl_student_visibility]]">
+ <a href="#" onclick="submit(event,
'?q=view_projects&prg=[[program_id]]&a=apply&c=student')"
+ class="btn btn-large [[dl_student_visibility]]">
<img src="[[skin_path]]/images/submit-project.png" width="26" height="26" alt="" />
{{apply_student}}
</a>
- <a href="?q=view_projects&prg=[[program_id]]&a=apply&c=mentor" class="btn btn-large
[[dl_mentor_visibility]]">
+ <a href="#" onclick="submit(event,
'?q=view_projects&prg=[[program_id]]&a=apply&c=mentor')"
+ class="btn btn-large [[dl_mentor_visibility]]">
<img src="[[skin_path]]/images/mentor-project.png" width="26" height="26" alt="" />
{{apply_mentor}}
</a>
diff --git a/skins/neverland/html/tpl_user_ban_item.html b/skins/neverland/html/tpl_user_ban_item.html
index 53ab396..2734609 100644
--- a/skins/neverland/html/tpl_user_ban_item.html
+++ b/skins/neverland/html/tpl_user_ban_item.html
@@ -4,8 +4,8 @@
</td>
<td>
- <a href="[[unban_url]]" title="{{unban_user}}">
+ <a href="#" onclick="submit(event, '[[unban_url]]')" title="{{unban_user}}">
<i class="icon-remove"></i>
</a>
</td>
-</tr>
\ No newline at end of file
+</tr>
diff --git a/skins/neverland/html/tpl_view_projects_item.html
b/skins/neverland/html/tpl_view_projects_item.html
index 459b2fe..ef13757 100644
--- a/skins/neverland/html/tpl_view_projects_item.html
+++ b/skins/neverland/html/tpl_view_projects_item.html
@@ -11,13 +11,13 @@
<td>
<div class="align-right [[apprej_visibility]]">
- <a href="[[approve_url]]" title="{{approve}}">
+ <a href="#" onclick="submit(event, '[[approve_url]]')" title="{{approve}}">
<i class="icon-ok"></i>
</a>
- <a href="[[reject_url]]" title="{{reject}}">
+ <a href="#" onclick="submit(event, '[[reject_url]]')" title="{{reject}}">
<i class="icon-remove"></i>
</a>
</div>
</td>
-</tr>
\ No newline at end of file
+</tr>
diff --git a/skins/neverland/js/main.js b/skins/neverland/js/main.js
index a578c23..8ea4893 100644
--- a/skins/neverland/js/main.js
+++ b/skins/neverland/js/main.js
@@ -4,6 +4,19 @@
* @copyright (c) 2012 KDE. All rights reserved.
*/
+function submit(evt, url) {
+ evt.preventDefault();
+ var form = document.getElementsByTagName("form")[0];
+ var oldAction = form.action;
+ form.action = url;
+ try {
+ form.submit();
+ } catch(e) {
+ form.action = oldAction;
+ thow(e);
+ }
+}
+
// Startup function
$(document).ready(function() {
$("#block-link").removeClass("hidden");
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]