MDL Shield
Sample Report— This is a fictitious plugin created to demonstrate the format and detail of an MDL Shield security review. The plugin, findings, and code samples shown here are entirely fabricated for illustrative purposes.

Quiz Analytics Dashboard (Fictitious)

local_fakequizanalytics

Plugin Information

This is a fictitious plugin used to demonstrate what an MDL Shield security review looks like. This plugin does not exist.

Version:2024031500
Release:2.1.0
Moodle:4.1 - 4.5
Reviewed:2024-03-15
47 files·8,420 lines
Human Verified
1
Critical
2
High
2
Medium
2
Low
1
Info
AI Summary

This plugin provides quiz analytics functionality with generally solid code structure. However, the security review identified several areas of concern, primarily around SQL query construction and output encoding. The most critical issues involve potential SQL injection vulnerabilities in the reporting module and missing capability checks on AJAX endpoints. The codebase follows Moodle coding standards reasonably well, but would benefit from consistent use of the Moodle database API and output functions.

Findings

CriticalVerified
SQL Injection in Report Filter

User-supplied filter parameters are concatenated directly into SQL query without proper sanitization or use of bound parameters.

classes/report/analytics_report.php:142
Vulnerable Code
$sql = "SELECT * FROM {quiz_attempts}
WHERE userid = " . $userid . "
AND quiz IN (" . $quizids . ")";
$results = $DB->get_records_sql($sql);
Recommended Fix
$sql = "SELECT * FROM {quiz_attempts}
WHERE userid = :userid
AND quiz IN ($quizids)";
$params = ['userid' => $userid];
// Use $DB->get_in_or_equal() for quiz IDs
list($insql, $inparams) = $DB->get_in_or_equal($quizids, SQL_PARAMS_NAMED);
$sql = "SELECT * FROM {quiz_attempts} WHERE userid = :userid AND quiz $insql";
$results = $DB->get_records_sql($sql, array_merge($params, $inparams));
Human Reviewer Note

Confirmed critical vulnerability. This is exploitable and must be fixed before release. The suggested fix correctly uses parameterized queries.

HighVerified
Missing Capability Check on AJAX Endpoint

The AJAX endpoint for fetching student data does not verify the user has the required capability before returning sensitive information.

ajax/get_student_data.php:28
Vulnerable Code
require_login();
$userid = required_param('userid', PARAM_INT);
$data = get_student_analytics($userid);
echo json_encode($data);
Recommended Fix
require_login();
$userid = required_param('userid', PARAM_INT);
$context = context_course::instance($courseid);
require_capability('local/quizanalytics:viewstudentdata', $context);
$data = get_student_analytics($userid);
echo json_encode($data);
Human Reviewer Note

Verified. Any authenticated user could access any student's analytics data. This is a significant privacy issue.

HighVerified
Cross-Site Scripting (XSS) in Dashboard Output

User-supplied quiz names are output without proper encoding, allowing potential XSS attacks.

classes/output/dashboard_renderer.php:89
Vulnerable Code
$html .= '<h3>' . $quiz->name . '</h3>';
$html .= '<p>' . $quiz->intro . '</p>';
Recommended Fix
$html .= '<h3>' . format_string($quiz->name) . '</h3>';
$html .= '<p>' . format_text($quiz->intro, FORMAT_HTML) . '</p>';
Human Reviewer Note

Confirmed. While quiz names are typically set by teachers, this should still use proper output encoding for defense in depth.

MediumVerified
Missing Sesskey Validation

Form submission handler does not validate the sesskey, potentially allowing CSRF attacks.

settings_save.php:15
Vulnerable Code
$action = optional_param('action', '', PARAM_ALPHA);
if ($action === 'save') {
    save_plugin_settings($data);
    redirect($returnurl);
}
Recommended Fix
$action = optional_param('action', '', PARAM_ALPHA);
if ($action === 'save') {
    require_sesskey();
    save_plugin_settings($data);
    redirect($returnurl);
}
Human Reviewer Note

Confirmed. Standard Moodle CSRF protection pattern should be applied.

MediumVerified
Potential Path Traversal in Export

File export functionality does not properly sanitize the filename parameter.

export.php:52
Vulnerable Code
$filename = required_param('filename', PARAM_TEXT);
$filepath = $CFG->tempdir . '/' . $filename;
file_put_contents($filepath, $content);
Recommended Fix
$filename = required_param('filename', PARAM_FILE);
$filename = clean_filename($filename);
$filepath = $CFG->tempdir . '/' . $filename;
file_put_contents($filepath, $content);
Human Reviewer Note

Verified. Using PARAM_FILE and clean_filename() prevents directory traversal.

LowDisputed
Hardcoded Database Table Prefix

Direct reference to 'mdl_' prefix instead of using Moodle's table naming conventions.

db/install.php:34
Vulnerable Code
$sql = "CREATE INDEX mdl_quizanalytics_idx ON mdl_local_fakequizanalytics (userid)";
Recommended Fix
// Use XMLDB or $DB methods instead
$table = new xmldb_table('local_fakequizanalytics');
$index = new xmldb_index('userid_idx', XMLDB_INDEX_NOTUNIQUE, ['userid']);
$dbman->add_index($table, $index);
Human Reviewer Note

False positive. This code is in a legacy migration script that only runs on specific older installations. The current install.xml handles this correctly. No action needed.

LowVerified
Deprecated Function Usage

Using deprecated get_context_instance() function instead of context_course::instance().

lib.php:78
Vulnerable Code
$context = get_context_instance(CONTEXT_COURSE, $courseid);
Recommended Fix
$context = context_course::instance($courseid);
Human Reviewer Note

Confirmed. This deprecated function was removed in Moodle 4.0. Must be updated for compatibility.

InfoVerified
Consider Using Mustache Templates

HTML generation in PHP could be moved to Mustache templates for better separation of concerns.

classes/output/dashboard_renderer.php:45
Vulnerable Code
$html = '<div class="analytics-card">';
$html .= '<h4>' . format_string($title) . '</h4>';
$html .= '<div class="stats">' . $stats . '</div>';
$html .= '</div>';
return $html;
Recommended Fix
// In templates/analytics_card.mustache:
// <div class="analytics-card">
//     <h4>{{title}}</h4>
//     <div class="stats">{{{stats}}}</div>
// </div>

return $this->render_from_template('local_fakequizanalytics/analytics_card', [
    'title' => format_string($title),
    'stats' => $stats,
]);
Human Reviewer Note

Good suggestion for maintainability, but not a security issue. Consider for future refactoring.

Additional AI Notes
  • The plugin would benefit from adding PHPUnit tests, particularly for the analytics calculation functions.
  • Consider implementing caching for expensive database queries in the dashboard rendering.
  • Documentation for capability requirements would help administrators understand the access control model.

The report above is entirely fictitious and for demonstration purposes only. No real plugin was reviewed.

Ready to secure your plugin?

Get the same thorough analysis for your Moodle plugin.