Skip to content

security and visual fixes for Moodle 5.x #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Syxton merged 1 commit into from
Jul 1, 2025
Merged

security and visual fixes for Moodle 5.x #70

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 71 additions & 43 deletions classes/processor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,7 @@ public static function save_state($stepid, $title, $data) {

$record = new stdClass();
$record->title = $title;
$record->content = serialize($data);
$record->content = json_encode($data);
$record->step = $stepid;
$record->savedate = $date->getTimestamp();
if ($DB->insert_record('tool_coursearchiver_saves', $record)) {
Expand Down Expand Up @@ -1070,11 +1070,16 @@ public static function get_saves() {
* @return string
*/
public static function get_savestatelist() {
global $CFG, $DB, $OUTPUT;
global $DB, $OUTPUT;

// Back button.
$savelist = html_writer::link(new moodle_url('/admin/tool/coursearchiver/index.php'),
get_string('back'));

$savelist .= html_writer::tag('h3',
get_string('savestatelist', 'tool_coursearchiver'),
['style' => 'text-align: center']);

// Table.
$savelist .= html_writer::start_tag('table', ['style' => 'border-collapse: collapse;width: 100%;',
'cellpadding' => '5',
Expand All @@ -1099,7 +1104,7 @@ public static function get_savestatelist() {
if ($saves) {
foreach ($saves as $savepoint) {
// Create security key for each link.
$key = sha1($CFG->dbpass . $savepoint->id);
$key = sha1(self::get_coursearchiver_keyid() . $savepoint->id);

$link = new moodle_url('/admin/tool/coursearchiver/removesavepoint.php',
['savepointid' => $savepoint->id, 'key' => $key]);
Expand Down Expand Up @@ -1352,6 +1357,21 @@ public function get_list_of_admins_and_managers() {
return $adminsandmanagers;
}

/**
* Get a unique server id to serve as the key. ID must not change between key creation and use.
*
* @return string
*/
public static function get_coursearchiver_keyid() {
global $DB;

return $DB->get_field(
"config_plugins",
"id",
["plugin" => "tool_coursearchiver", "name" => "version"],
);
}

/**
* Static method to get list of the userid's of admin and other users with course view capability
*
Expand All @@ -1370,8 +1390,6 @@ public static function get_list_of_admins_and_managers_static() {
* @return array Full HTML table listing the $courses
*/
public function get_email_courses($obj, $links = true) {
global $CFG;

if ($this->mode == self::MODE_HIDEEMAIL) {
$optoutbutton = get_string('optouthide', 'tool_coursearchiver');
} else if ($this->mode == self::MODE_ARCHIVEEMAIL) {
Expand All @@ -1387,7 +1405,7 @@ public function get_email_courses($obj, $links = true) {
$rowcolor = "#FFF";
foreach ($obj["courses"] as $course) {
// Create security key for each link.
$key = sha1($CFG->dbpass . $course->id . $obj["user"]->id);
$key = sha1(self::get_coursearchiver_keyid() . $course->id . $obj["user"]->id);

// Only add courses that are not hidden if mode is HIDEEMAIL.
if ($this->mode == self::MODE_HIDEEMAIL && !$course->visible) {
Expand Down Expand Up @@ -1493,41 +1511,43 @@ public static function optout_course($courseid, $userid) {
* @return string
*/
public static function get_optoutlist() {
global $CFG, $DB, $OUTPUT;
global $DB, $OUTPUT;

$sql = "SELECT *
FROM {tool_coursearchiver_optout}
ORDER BY optouttime";
$sql = "SELECT * FROM {tool_coursearchiver_optout} ORDER BY optouttime";
$optouts = $DB->get_records_sql($sql);

// Back button.
$courses = html_writer::link(new moodle_url('/admin/tool/coursearchiver/index.php'),
get_string('back'));
$courses = html_writer::link(new moodle_url('/admin/tool/coursearchiver/index.php'), get_string('back'));

$courses .= html_writer::tag('h3',
get_string('optoutlist', 'tool_coursearchiver'),
['style' => 'text-align: center']);

// Archive table.
$courses .= html_writer::start_tag('table', ['style' => 'border-collapse: collapse;width: 100%;',
'cellpadding' => '5',
]);
$rowcolor = "#FFF";
$courses .= html_writer::tag('tr',
html_writer::tag('th',
get_string('course')) .
html_writer::tag('th',
get_string('optouttime', 'tool_coursearchiver'),
['style' => 'text-align: center']) .
html_writer::tag('th',
get_string('optoutby', 'tool_coursearchiver'),
['style' => 'text-align: center']) .
html_writer::tag('th',
get_string('actions'),
['width' => '100px', 'style' => 'text-align: center']),
['style' => 'background-color:' . $rowcolor]);
html_writer::tag('th',
get_string('course')) .
html_writer::tag('th',
get_string('optouttime', 'tool_coursearchiver'),
['style' => 'text-align: center']) .
html_writer::tag('th',
get_string('optoutby', 'tool_coursearchiver'),
['style' => 'text-align: center']) .
html_writer::tag('th',
get_string('actions'),
['width' => '100px', 'style' => 'text-align: center']),
['style' => 'background-color:' . $rowcolor]);

if ($optouts) {
foreach ($optouts as $optout) {
$user = $DB->get_record('user', ['id' => $optout->userid]);
if ($course = $DB->get_record('course', ['id' => $optout->courseid], '*', IGNORE_MISSING)) {
// Create security key for each link.
$key = sha1($CFG->dbpass . $course->id . $optout->userid);
$key = sha1(self::get_coursearchiver_keyid() . $course->id . $optout->userid);

if ($optout->optoutlength == 0) {
$ago = "∞";
Expand Down Expand Up @@ -1609,31 +1629,39 @@ public static function get_archivelist($search, $recover = false) {
"/\\");
// Form start.
$rowcolor = "#FFF";
$data = ["formstart" => true,
"isadmin" => $isadmin,
"recover" => $recover,
"searchterm" => $search,
"rowcolor" => $rowcolor,
"limiter" => $config->archivelimit,
];
$data = [
"formstart" => true,
"isadmin" => $isadmin,
"recover" => $recover,
"searchterm" => $search,
"rowcolor" => $rowcolor,
"limiter" => $config->archivelimit,
];
$courses = $OUTPUT->render_from_template('tool_coursearchiver/archive_view', $data);

$params = [];

// Get either archives that are not marked for deletion or those that have been.
$select = !$recover ? 'timetodelete = 0' : 'timetodelete > 0';
$sql = !$recover ? 'timetodelete = 0' : 'timetodelete > 0';

// Search criteria.
$select .= !empty($search) ? " AND filename LIKE '%$search%'" : '';
$params['filename'] = '%' . $DB->sql_like_escape($search) . '%';
$sql .= !empty($search) ? " AND " . $DB->sql_like("filename", ":filename", false, false) : '';

// Only show user files.
$select .= $isadmin ? '' : " AND owners LIKE '%|$USER->id|%'";

$archives = $DB->get_records_select('tool_coursearchiver_archived',
$select,
null,
'filename',
'*',
0,
$config->archivelimit);
$params['owners'] = '%|' . $DB->sql_like_escape($USER->id) . '|%';
$sql .= $isadmin ? '' : " AND " . $DB->sql_like("owners", ":owners", false, false);

$archives = $DB->get_records_select(
'tool_coursearchiver_archived',
$sql,
$params,
'filename',
'*',
0,
$config->archivelimit
);

if ($archives) {
foreach ($archives as $archive) {
$pathinfo = pathinfo($archive->filename);
Expand Down
4 changes: 2 additions & 2 deletions classes/step2_form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public function definition() {
$data = $this->_customdata['processor_data'];

$mform->addElement('hidden', 'formdata');
$mform->setType('formdata', PARAM_RAW);
$mform->setDefault('formdata', serialize($data['searches']));
$mform->setType('formdata', PARAM_TEXT);
$mform->setDefault('formdata', json_encode($data['searches']));

$mform->addElement('header', 'searchresultshdr', get_string('courseselector', 'tool_coursearchiver'));

Expand Down
4 changes: 2 additions & 2 deletions classes/step3_form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public function definition() {
$data = $this->_customdata['processor_data'];

$mform->addElement('hidden', 'formdata');
$mform->setType('formdata', PARAM_RAW);
$mform->setDefault('formdata', serialize($data['courses']));
$mform->setType('formdata', PARAM_TEXT);
$mform->setDefault('formdata', json_encode($data['courses']));

$mform->addElement('header', 'emaillist', get_string('emailselector', 'tool_coursearchiver'));

Expand Down
16 changes: 8 additions & 8 deletions classes/step4_form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ public function definition() {
$data = $this->_customdata['processor_data'];

$mform->addElement('hidden', 'formdata');
$mform->setType('formdata', PARAM_RAW);
$mform->setType('formdata', PARAM_TEXT);
$mform->setDefault('formdata', $data['formdata']);

$mform->addElement('hidden', 'mode');
$mform->setType('mode', PARAM_INT);
$mform->setDefault('mode', $data['mode']);
$mform->addElement('hidden', 'coursearchiver_mode');
$mform->setType('coursearchiver_mode', PARAM_INT);
$mform->setDefault('coursearchiver_mode', $data['mode']);

$count = count(unserialize($data["formdata"]));
$count = count(json_decode($data["formdata"]));
if (empty($count)) {
$returnurl = new moodle_url('/admin/tool/coursearchiver/index.php',
["error" => get_string('unknownerror', 'tool_coursearchiver')]);
Expand All @@ -57,23 +57,23 @@ public function definition() {

switch($data["mode"]) {
case tool_coursearchiver_processor::MODE_HIDEEMAIL:
foreach (unserialize($data["formdata"]) as $r) { // Loop through every possible user.
foreach (json_decode($data["formdata"]) as $r) { // Loop through every possible user.
if (substr($r, 0, 1) == 'x') { // Determine if they were NOT selected.
$count--; // Remove 1 from count.
}
}
$message = get_string('confirmmessagehideemail', 'tool_coursearchiver', $count);
break;
case tool_coursearchiver_processor::MODE_ARCHIVEEMAIL:
foreach (unserialize($data["formdata"]) as $r) { // Loop through every possible user.
foreach (json_decode($data["formdata"]) as $r) { // Loop through every possible user.
if (substr($r, 0, 1) == 'x') { // Determine if they were NOT selected.
$count--; // Remove 1 from count.
}
}
$message = get_string('confirmmessagearchiveemail', 'tool_coursearchiver', $count);
break;
case tool_coursearchiver_processor::MODE_DELETEEMAIL:
foreach (unserialize($data["formdata"]) as $r) { // Loop through every possible user.
foreach (json_decode($data["formdata"]) as $r) { // Loop through every possible user.
if (substr($r, 0, 1) == 'x') { // Determine if they were NOT selected.
$count--; // Remove 1 from count.
}
Expand Down
2 changes: 1 addition & 1 deletion classes/tracker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ public function start() {
* @return void
*/
public function output($data, $info = false) {
global $CFG, $DB;
global $CFG;

$return = 1; // By default we are returning the a single process as finished.

Expand Down
16 changes: 16 additions & 0 deletions db/upgrade.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,5 +204,21 @@ function xmldb_tool_coursearchiver_upgrade($oldversion) {
upgrade_plugin_savepoint(true, 2020022700, 'tool', 'coursearchiver');
}

// Update saves table to use json_encoding instead of serialize.
if ($oldversion < 2025070102) {
if ($saves = $DB->get_records('tool_coursearchiver_saves')) {
foreach ($saves as $save) {
$content = unserialize($save->content);
if (!empty($content)) {
$save->content = json_encode($content);
$DB->update_record('tool_coursearchiver_saves', $save);
}
}
}

// Monitor savepoint reached.
upgrade_plugin_savepoint(true, 2025070102, 'tool', 'coursearchiver');
}

return true;
}
31 changes: 17 additions & 14 deletions index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,38 +33,41 @@
admin_externalpage_setup('toolcoursearchiver');

global $SESSION;
$error = isset($SESSION->error) ? $SESSION->error : optional_param('error', false, PARAM_RAW);
$submitted = optional_param('submitbutton', false, PARAM_RAW);
$error = $SESSION->coursearchiver_error ?? optional_param('coursearchiver_error', false, PARAM_TEXT);
$error = htmlspecialchars($error, ENT_COMPAT);

$submitted = optional_param('submitbutton', false, PARAM_TEXT);
$submitted = htmlspecialchars($submitted, ENT_COMPAT);

// Button to start over has been pressed.
if ($submitted == get_string('back', 'tool_coursearchiver')) {
unset($SESSION->formdata);
unset($SESSION->mode);
unset($SESSION->error);
if ($submitted == htmlspecialchars(get_string('back', 'tool_coursearchiver'), ENT_COMPAT)) {
unset($SESSION->coursearchiver_formdata);
unset($SESSION->coursearchiver_mode);
unset($SESSION->coursearchiver_error);
unset($SESSION->selected);
$returnurl = new moodle_url('/admin/tool/coursearchiver/index.php');
redirect($returnurl);
}

// View optouts list button.
if (!empty($submitted)) {
if ($submitted == get_string('optoutlist', 'tool_coursearchiver')) {
if ($submitted == htmlspecialchars(get_string('optoutlist', 'tool_coursearchiver'), ENT_COMPAT)) {
$returnurl = new moodle_url('/admin/tool/coursearchiver/optoutlist.php');
redirect($returnurl);
}

if ($submitted == get_string('archivelist', 'tool_coursearchiver')) {
if ($submitted == htmlspecialchars(get_string('archivelist', 'tool_coursearchiver'), ENT_COMPAT)) {
$returnurl = new moodle_url('/admin/tool/coursearchiver/archivelist.php');
redirect($returnurl);
}

if ($submitted == get_string('savestatelist', 'tool_coursearchiver')) {
if ($submitted == htmlspecialchars(get_string('savestatelist', 'tool_coursearchiver'), ENT_COMPAT)) {
$returnurl = new moodle_url('/admin/tool/coursearchiver/savestatelist.php');
redirect($returnurl);
}
}

unset($SESSION->error);
unset($SESSION->coursearchiver_error);

$mform = new tool_coursearchiver_step1_form(null);

Expand All @@ -78,12 +81,12 @@
if (!empty($formdata->savestates)) {
$formdata->searches["savestates"] = $formdata->savestates;
if ($save = tool_coursearchiver_processor::get_save($formdata->savestates)) {
$SESSION->formdata = $save->content;
$SESSION->resume = true;
$SESSION->coursearchiver_formdata = $save->content;
$SESSION->coursearchiver_resume = true;
$returnurl = new moodle_url('/admin/tool/coursearchiver/step'.$save->step.'.php');
redirect($returnurl);
} else {
$SESSION->error = get_string('unknownerror', 'tool_coursearchiver');
$SESSION->coursearchiver_error = get_string('unknownerror', 'tool_coursearchiver');
$returnurl = new moodle_url('/admin/tool/coursearchiver/index.php');
redirect($returnurl);
}
Expand Down Expand Up @@ -139,7 +142,7 @@
$formdata->searches["subcats"] = true;
}

$SESSION->formdata = serialize($formdata->searches);
$SESSION->coursearchiver_formdata = json_encode($formdata->searches);
$returnurl = new moodle_url('/admin/tool/coursearchiver/step2.php');
redirect($returnurl);
} else { // Form 1 data did not come across correctly.
Expand Down
4 changes: 2 additions & 2 deletions optin.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
define('NO_OUTPUT_BUFFERING', true);

require(__DIR__ . '/../../../config.php');
require_once($CFG->libdir.'/adminlib.php');
require_once($CFG->libdir . '/adminlib.php');

header('X-Accel-Buffering: no');

Expand All @@ -46,7 +46,7 @@
echo $OUTPUT->heading_with_help(get_string('coursearchiver', 'tool_coursearchiver'), 'coursearchiver', 'tool_coursearchiver');

// Check to see if the attempt is coming from a valid email.
if (sha1($CFG->dbpass . $courseid . $userid) == $key) {
if (sha1(tool_coursearchiver_processor::get_coursearchiver_keyid() . $courseid . $userid) == $key) {
if ($course = get_course($courseid)) {
$date = new DateTime("now", core_date::get_user_timezone_object());
$optouttime = $date->getTimestamp();
Expand Down
Loading
Loading