Skip to content

Commit

Permalink
Fixed CSRF vulnerabilities for high-impact administrator actions. Als…
Browse files Browse the repository at this point in the history
…o some minor refactoring of the naming of the CSRF token.
  • Loading branch information
Per Wammer committed Jan 9, 2017
1 parent 75df186 commit 9c11dee
Show file tree
Hide file tree
Showing 40 changed files with 88 additions and 27 deletions.
2 changes: 2 additions & 0 deletions admin/config_edit.php
Expand Up @@ -22,6 +22,8 @@
header('Location: index.php');
exit;
} else if (isset($_POST['submit'])) {
check_csrf_token();

$missing_fields = array();

$_POST['site_name'] = trim($_POST['site_name']);
Expand Down
13 changes: 13 additions & 0 deletions include/lib/vital_funcs.inc.php
Expand Up @@ -680,6 +680,19 @@ function check_referer(){
}
}
}
/**
* Check if token supplied in a POST request corresponds to the token in memory to prevent CSRF access
* @access public
* @return error message access denied
*/
function check_csrf_token() {
global $msg;
if($_POST['csrftoken'] != $_SESSION['token']){
$msg->addError('ACCESS_DENIED');
header('Location: '.AT_BASE_HREF.'index.php');
exit;
}
}
/**
* Check if the give theme is a subsite customized theme. Return true if it is, otherwise, return false
* @access public
Expand Down
2 changes: 2 additions & 0 deletions mods/_core/courses/admin/create_course.php
Expand Up @@ -23,6 +23,8 @@
header('Location: '.AT_BASE_HREF.'mods/_core/courses/admin/courses.php');
exit;
} else if (isset($_POST['form_course'])) {
check_csrf_token();

$errors = add_update_course($_POST, TRUE);
if ($errors !== FALSE) {
$msg->addFeedback('ACTION_COMPLETED_SUCCESSFULLY');
Expand Down
7 changes: 2 additions & 5 deletions mods/_core/courses/users/create_course.php
Expand Up @@ -68,12 +68,9 @@
header('Location: index.php');
exit;
}else if (isset($_POST['form_course']) && $_POST['submit'] != '') {
check_csrf_token();

$_POST['instructor'] = $_SESSION['member_id'];
if($_POST['csrftoken'] != $_SESSION['token']){
$msg->addError('ACCESS_DENIED');
header('Location: '.AT_BASE_HREF.'index.php');
exit;
}
$errors = add_update_course($_POST);

if ($errors !== FALSE) {
Expand Down
7 changes: 7 additions & 0 deletions mods/_core/enrolment/html/enroll_edit.inc.php
Expand Up @@ -234,20 +234,26 @@ function alumni ($list) {
exit;
}
else if (isset($_POST['submit_yes']) && $_POST['func'] =='unenroll' ) {
check_csrf_token();

//Unenroll student from course
unenroll($_POST['id']);

$msg->addFeedback('MEMBERS_REMOVED');
header('Location: index.php?current_tab=4'.SEP.'course_id='.$course_id);
exit;
} else if (isset($_POST['submit_yes']) && $_POST['func'] =='enroll' ) {
check_csrf_token();

//Enroll student in course
enroll($_POST['id']);

$msg->addFeedback('MEMBERS_ENROLLED');
header('Location: index.php?current_tab=0'.SEP.'course_id='.$course_id);
exit;
} else if (isset($_POST['submit_yes']) && $_POST['func'] =='alumni' ) {
check_csrf_token();

//Mark student as course alumnus
alumni($_POST['id']);

Expand Down Expand Up @@ -289,6 +295,7 @@ function alumni ($list) {
$hidden_vars['current_tab'] = $_GET['current_tab'];
$hidden_vars['gid'] = abs($_GET['gid']);
$hidden_vars['course_id'] = $course_id;
$hidden_vars['csrftoken'] = $_SESSION['token'];
//get usernames of users about to be edited
$str = get_usernames($member_ids);

Expand Down
3 changes: 3 additions & 0 deletions mods/_core/properties/admin/delete_course.php
Expand Up @@ -22,6 +22,8 @@
header('Location: ../../courses/admin/courses.php');
exit;
} else if (isset($_POST['step']) && ($_POST['step'] == 2) && isset($_POST['submit_yes'])) {
check_csrf_token();

require_once(AT_INCLUDE_PATH.'../mods/_core/file_manager/filemanager.inc.php');
require(AT_INCLUDE_PATH.'../mods/_core/properties/lib/delete_course.inc.php');

Expand All @@ -43,6 +45,7 @@
} else if ($_POST['step'] == 1) {
$hidden_vars['step'] = 2;
$hidden_vars['course'] = $course;
$hidden_vars['csrftoken'] = $_SESSION['token'];
$msg->addConfirm(array('DELETE_COURSE_2', $system_courses[$course]['title']), $hidden_vars);
$msg->printConfirm();
}
Expand Down
2 changes: 2 additions & 0 deletions mods/_core/properties/admin/edit_course.php
Expand Up @@ -22,6 +22,8 @@
header('Location: ../../courses/admin/courses.php');
exit;
} else if (isset($_POST['submit'])) {
check_csrf_token();

require(AT_INCLUDE_PATH.'../mods/_core/courses/lib/course.inc.php');
$errors = add_update_course($_POST, TRUE);

Expand Down
4 changes: 3 additions & 1 deletion mods/_core/users/admin_delete.php
Expand Up @@ -147,7 +147,8 @@ function delete_user($id) {
$ids = explode(',', $_REQUEST['id']);

if (isset($_POST['submit_yes'])) {

check_csrf_token();

foreach($ids as $id) {
delete_user(intval($id));
}
Expand All @@ -174,6 +175,7 @@ function delete_user($id) {
$names_html = '<ul>'.html_get_list($names).'</ul>';
$hidden_vars['id'] = implode(',', array_keys($names));
$hidden_vars['ml'] = intval($_REQUEST['ml']);
$hidden_vars['csrftoken'] = $_SESSION['token'];

$confirm = array('DELETE_USER', $names_html);
$msg->addConfirm($confirm, $hidden_vars);
Expand Down
2 changes: 2 additions & 0 deletions mods/_core/users/admin_email.php
Expand Up @@ -21,6 +21,8 @@
header('Location: users.php#feedback');
exit;
} else if ($_POST['submit']) {
check_csrf_token();

$missing_fields = array();

$_POST['subject'] = trim($_POST['subject']);
Expand Down
3 changes: 3 additions & 0 deletions mods/_core/users/admins/delete.php
Expand Up @@ -23,6 +23,8 @@
header('Location: index.php');
exit;
} else if (isset($_POST['submit_yes'])) {
check_csrf_token();

$_POST['login'] = $addslashes($_POST['login']);

$sql = "DELETE FROM %sadmins WHERE login='%s'";
Expand Down Expand Up @@ -52,6 +54,7 @@
echo _AT('no_user_found');
} else {
$hidden_vars['login'] = $_GET['login'];
$hidden_vars['csrftoken'] = $_SESSION['token'];
$confirm = array('DELETE_ADMIN', $row_admins['login']);
$msg->addConfirm($confirm, $hidden_vars);
$msg->printConfirm();
Expand Down
2 changes: 2 additions & 0 deletions mods/_core/users/admins/edit.php
Expand Up @@ -27,6 +27,8 @@
header('Location: index.php');
exit;
} else if (isset($_POST['submit'])) {
check_csrf_token();

$missing_fields = array();

/* email validation */
Expand Down
1 change: 1 addition & 0 deletions mods/_core/users/admins/password.php
Expand Up @@ -20,6 +20,7 @@
header('Location: '.AT_BASE_HREF.'mods/_core/users/admins/index.php');
exit;
} else if (isset($_POST['submit'])) {
check_csrf_token();
/* password check: password is verified front end by javascript. here is to handle the errors from javascript */
if ($_POST['password_error'] <> "")
{
Expand Down
2 changes: 2 additions & 0 deletions mods/_core/users/admins/reset_log.php
Expand Up @@ -22,6 +22,7 @@
header('Location: ./log.php');
exit;
} else if (isset($_POST['submit_yes'])) {
check_csrf_token();
//clean up the db
$sql = "DELETE FROM %sadmin_log";
$result = queryDB($sql, array(TABLE_PREFIX));
Expand All @@ -37,6 +38,7 @@

//print confirmation
$hidden_vars['all'] = TRUE;
$hidden_vars['csrftoken'] = $_SESSION['token'];

$confirm = array('RESET_ADMIN_LOG', $_SERVER['PHP_SELF']);
$msg->addConfirm($confirm, $hidden_vars);
Expand Down
2 changes: 2 additions & 0 deletions mods/_core/users/edit_user.php
Expand Up @@ -28,6 +28,8 @@
}

if (isset($_POST['submit'])) {
check_csrf_token();

$missing_fields = array();

$id = intval($_POST['id']);
Expand Down
13 changes: 7 additions & 6 deletions mods/_core/users/instructor_requests.php
Expand Up @@ -18,13 +18,14 @@
require(AT_INCLUDE_PATH.'vitals.inc.php');
admin_authenticate(AT_ADMIN_PRIV_USERS);

if (isset($_GET['deny']) && isset($_GET['id'])) {
header('Location: admin_deny.php?id='.$_GET['id']);
if (isset($_POST['deny']) && isset($_POST['id'])) {
header('Location: admin_deny.php?id='.$_POST['id']);
exit;

} else if (isset($_GET['approve']) && isset($_GET['id'])) {

$id = intval($_GET['id']);
} else if (isset($_POST['approve']) && isset($_POST['id'])) {
check_csrf_token();

$id = intval($_POST['id']);

$sql = 'DELETE FROM %sinstructor_approvals WHERE member_id=%d';
$result = queryDB($sql, array(TABLE_PREFIX, $id));
Expand Down Expand Up @@ -69,7 +70,7 @@
}

$msg->addFeedback('PROFILE_UPDATED_ADMIN');
} else if (!empty($_GET) && !$_GET['submit']) {
} else if (!empty($_POST) && !$_POST['submit']) {
$msg->addError('NO_ITEM_SELECTED');
}

Expand Down
1 change: 1 addition & 0 deletions mods/_core/users/password_user.php
Expand Up @@ -23,6 +23,7 @@
header('Location: '.AT_BASE_HREF.'mods/_core/users/users.php');
exit;
} else if (isset($_POST['submit'])) {
check_csrf_token();
/* password check: password is verified front end by javascript. here is to handle the errors from javascript */
if ($_POST['password_error'] <> "")
{
Expand Down
2 changes: 2 additions & 0 deletions mods/_standard/basiclti/tool/admin_create.php
Expand Up @@ -35,6 +35,7 @@
header('Location: '.AT_BASE_HREF.'mods/_standard/basiclti/index_admin.php');
exit;
} else if (isset($_POST['form_basiclti'])) {
check_csrf_token();

if ( at_form_validate($blti_admin_form, $msg) ) {

Expand Down Expand Up @@ -65,6 +66,7 @@

?>
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" name="basiclti_form" enctype="multipart/form-data">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<input type="hidden" name="form_basiclti" value="true" />
<div class="input-form">
<fieldset class="group_form"><legend class="group_form"><?php echo _AT('properties'); ?></legend>
Expand Down
2 changes: 2 additions & 0 deletions mods/_standard/basiclti/tool/admin_delete.php
Expand Up @@ -23,6 +23,7 @@
header('Location: ../index_admin.php');
exit;
} else if (isset($_POST['submit_yes'])) {
check_csrf_token();

$sql = "DELETE FROM %sbasiclti_tools WHERE id =%d";
$result = queryDB($sql, array(TABLE_PREFIX, $tool));
Expand All @@ -38,6 +39,7 @@
if (!isset($_POST['step'])) {
$hidden_vars['step'] = 2;
$hidden_vars['id'] = $tool;
$hidden_vars['csrftoken'] = $_SESSION['token'];
$msg->addConfirm(array('DELETE_TOOL_1', $row['title']), $hidden_vars);
$msg->printConfirm();
}
Expand Down
2 changes: 2 additions & 0 deletions mods/_standard/basiclti/tool/admin_edit.php
Expand Up @@ -36,6 +36,7 @@
header('Location: '.AT_BASE_HREF.'mods/_standard/basiclti/index_admin.php');
exit;
} else if (isset($_POST['form_basiclti'], $tool)) {
check_csrf_token();

if ( at_form_validate($blti_admin_form, $msg) ) {
$sql = "SELECT count(*) cnt FROM %sbasiclti_tools WHERE toolid = '%s' AND id != %d";
Expand Down Expand Up @@ -73,6 +74,7 @@
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" name="basiclti_form" enctype="multipart/form-data">
<input type="hidden" name="form_basiclti" value="true" />
<input type="hidden" name="id" value="<?php echo $tool; ?>" />
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<div class="input-form">
<fieldset class="group_form"><legend class="group_form"><?php echo _AT('properties'); ?></legend>
<?php at_form_generate($toolrow, $blti_admin_form); ?>
Expand Down
3 changes: 3 additions & 0 deletions mods/_standard/patcher/classes/Patch.class.php
Expand Up @@ -302,6 +302,7 @@ function checkPriviledge()
}

$notes = '<form action="'. $_SERVER['PHP_SELF'].'?id='.$id.'&who='. $who .'" method="post" name="skip_files_modified">
<input type="hidden" name="csrftoken" value="'.$_SESSION['token'].'" />
<div class="row buttons">
<input type="submit" name="yes" value="'._AT('continue').'" accesskey="y" />
<input type="submit" name="no" value="'. _AT('cancel'). '" />
Expand Down Expand Up @@ -337,6 +338,7 @@ function checkAppliedVersion()

$notes = '
<form action="'. $_SERVER['PHP_SELF'].'?id='.$_POST['id'].'&who='. $_POST['who'] .'" method="post" name="skip_files_modified">
<input type="hidden" name="csrftoken" value="'.$_SESSION['token'].'" />
<div class="row buttons">
<input type="submit" name="ignore_version" value="'._AT('yes').'" accesskey="y" />
<input type="submit" name="not_ignore_version" value="'. _AT('no'). '" />
Expand Down Expand Up @@ -453,6 +455,7 @@ function hasFilesModified() {
else
$notes = '
<form action="'. $_SERVER['PHP_SELF'].'?id='.$_POST['id'].'&who='. $_POST['who'] .'" method="post" name="skip_files_modified">
<input type="hidden" name="csrftoken" value="'.$_SESSION['token'].'" />
<div class="row buttons">
<input type="submit" name="yes" value="'._AT('yes').'" accesskey="y" />
<input type="submit" name="no" value="'. _AT('no'). '" />
Expand Down
2 changes: 2 additions & 0 deletions mods/_standard/patcher/index_admin.php
Expand Up @@ -130,6 +130,7 @@ function get_array_by_delimiter($subject, $delimiter)
// Installation process
if ($_POST['install'] || $_POST['install_upload'] && !isset($_POST["not_ignore_version"]))
{
check_csrf_token();

if (isset($_POST['id'])) $id=$_POST['id'];
else $id = $_REQUEST['id'];
Expand Down Expand Up @@ -309,6 +310,7 @@ function get_array_by_delimiter($subject, $delimiter)
?>

<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post" name="form">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<div class="input-form">

<table class="data" summary="" style="width: 100%">
Expand Down
10 changes: 2 additions & 8 deletions registration.php
Expand Up @@ -34,6 +34,8 @@
header('Location: ./login.php');
exit;
} else if (isset($_POST['submit'])) {
check_csrf_token();

if(isset($_SESSION['member_id']) && $_SESSION['member_id'] > 0 && $_SESSION['login']) {
$member_id = $_SESSION['member_id'];
require (AT_INCLUDE_PATH.'html/auto_enroll_courses.inc.php');
Expand All @@ -52,14 +54,6 @@

$missing_fields = array();

/* registration token validation */
if (sha1($_SESSION['token']) != $_POST['registration_token']){
//Prevent registration from any other pages other than the ATutor pages.
//SHA1(SESSION[token]) so that no one knows what the actual token is, thus cannot recreate it on another page.
header('Location: ./login.php');
exit;
}

/* email check */
$chk_email = $addslashes($_POST['email']);
$chk_login = $addslashes($_POST['login']);
Expand Down
2 changes: 1 addition & 1 deletion themes/default/admin/courses/edit_course.tmpl.php
Expand Up @@ -4,6 +4,7 @@
?>
<?php //echo _AT('available_immediately'); ?>
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" name="course_form" enctype="multipart/form-data">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<input type="hidden" name="form_course" value="true" />
<input type="hidden" name="MAX_FILE_SIZE" value="<?php echo $_config['prof_pic_max_file_size']; ?>" />
<input type="hidden" name="course" value="<?php echo $this->course; ?>" />
Expand All @@ -12,7 +13,6 @@
<input type="hidden" name="show_courses" value="<?php echo intval($_GET['show_courses']); ?>" />
<input type="hidden" name="current_cat" value="<?php echo intval($_GET['current_cat']); ?>" />
<input type="submit" name="submit" style="display:none;"/>
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />

<div class="input-form">
<fieldset class="group_form"><legend class="group_form"><?php echo _AT('properties'); ?></legend>
Expand Down
@@ -1,6 +1,7 @@
<?php global $_config, $languageManager, $_config_defaults, $stripslashes;?>

<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" name="form">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<div class="input-form">
<div class="row">
<span class="required" title="<?php echo _AT('required_field'); ?>">*</span><label for="sitename"><?php echo _AT('site_name'); ?></label><br />
Expand Down
1 change: 1 addition & 0 deletions themes/default/admin/users/admin_email.tmpl.php
@@ -1,5 +1,6 @@

<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" name="form">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<input type="hidden" name="admin" value="admin" />

<div class="input-form">
Expand Down
1 change: 1 addition & 0 deletions themes/default/admin/users/edit.tmpl.php
@@ -1,4 +1,5 @@
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" name="form">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<input type="hidden" name="login" value="<?php echo $this->login; ?>" />
<input type="hidden" name="hide_email" value="<?php echo $this->hide_email; ?>" />
<div class="input-form">
Expand Down
3 changes: 2 additions & 1 deletion themes/default/admin/users/instructor_requests.tmpl.php
@@ -1,5 +1,6 @@

<form name="form" method="get" action="<?php echo $_SERVER['PHP_SELF']; ?>">
<form name="form" method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<table class="data" summary="Table listing instructor requests">
<thead>
<tr>
Expand Down
1 change: 1 addition & 0 deletions themes/default/admin/users/password.tmpl.php
@@ -1,4 +1,5 @@
<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" name="form">
<input type="hidden" name="csrftoken" value="<?php echo $_SESSION['token'];?>" />
<input type="hidden" name="login" value="<?php echo $this->row['login']; ?>" />
<input type="hidden" name="form_password_hidden" value="" />
<input type="hidden" name="password_error" value="" />
Expand Down

1 comment on commit 9c11dee

@fgeek
Copy link

@fgeek fgeek commented on 9c11dee Sep 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this CSRF issue have CVE identifier assigned?

Please sign in to comment.