Questionnaire
mod_questionnaire
- #1myreport.php trusts the group parameter, exposing other groups' feedback scores
- #2Recent-activity group filter uses wrong modinfo key and stale group cache
- #3Backup/restore drops progressbar, feedback-notes files and shared-survey mapping
- #4submit_questionnaire_response discards validate_parameters() result
- #5MySQL-only UPDATE ... JOIN syntax in legacy upgrade step
- #6Hard-coded English strings in mobile templates and PDF metadata
Questionnaire activity module: teachers build surveys (radio, checkbox, rate, text, essay, date, numeric, slider, file and other question types, with branching and feedback sections) that students complete. It provides response reports, CSV/dataformat and PDF exports, email and message notifications, non-respondent messaging, Moodle app support and public or template surveys that can be shared across courses.
The plugin is a large (~43k lines of non-test code), recently refactored activity module with a strong security posture. Every state-changing action is protected by require_sesskey(), confirm_sesskey(), moodleforms or the external API. Response IDs that come from the client are never trusted: they are re-derived on the server or checked with require_own_incomplete_response() and can_view_response(). Separate-groups isolation is enforced in report.php, file downloads and notifications. Respondent-uploaded files are always force-downloaded. Staff HTML goes through a correct trusted-text implementation. TCPDF input has remote images stripped. All SQL is parameterised.
The only finding with security relevance is low. In myreport.php, a student can pass an arbitrary group id. When a separate-groups questionnaire shows feedback score tables or charts, this reveals another group's aggregate feedback percentages and the name of any group on the site. No individual responses or identities are exposed, and several configuration conditions must hold, so this caps the grade just below the top tier rather than lower.
The remaining findings are code-quality issues:
- a fatal error in the recent-activity group filter
- backup/restore gaps (a lost setting, unannotated files, an unmapped shared-survey id)
- discarded web-service parameter validation
- one MySQL-only statement in a legacy upgrade step
- a few hard-coded English strings
None of these is exploitable.
mod_questionnaire is a large, mature survey activity: about 43k lines of PHP, mustache and AMD JavaScript outside tests. It covers a dozen question types, branching, feedback scoring, CSV/PDF exports, email and message notifications, Moodle app support and surveys shared across courses. The codebase has clearly been through a deliberate security hardening pass. Permissions are centralised in a capabilities class. Every response and file access goes through can_view_response(), which checks questionnaire ownership, group visibility and individual-response rights. Client-supplied response ids are re-derived on the server, uploads are type-checked on the server and always force-downloaded, and staff HTML is handled by a careful trusted-text wrapper around format_text().
I found no SQL injection, stored XSS, CSRF or authorisation bypass in the main flows. Those flows are: completing a questionnaire on the web or in the app, reports, downloads, deletion, question editing, feedback editing, non-respondent messaging and the two web services. Every template that renders student input escapes it, and student free text is passed through clean_text() before storage and format_text() on display.
The one finding worth acting on is a group-isolation gap in myreport.php: unlike report.php, it does not validate the group parameter. In separate-groups mode with feedback score tables or charts enabled, a student can see another group's aggregate feedback percentages and the name of any group on the site. The impact is small, and the fix is to reuse the existing can_view_group() check.
The other findings are low-severity code quality issues. They include a fatal error in the recent-activity filter for separate-groups graders, and backup/restore omissions that silently drop a setting and the feedback-notes images.
Findings
myreport.php reads group with optional_param(). In separate-groups mode it only replaces the value when it is 0, so any non-zero group id survives, including one the student is not a member of or one from another course. In the vresp action that id is passed to get_responses(false, $currentgroupid) and on to view_response(... $compare = true ...). feedback::build_scoreboard() then prints that group's aggregate feedback percentages, along with the group name from groups_get_group_name(), in the score table and chart title. report.php already blocks this with capabilities::can_view_group(), but myreport.php never calls it.
Low risk. Only aggregate percentages and group names are exposed; no individual answers or identities. Several conditions must all hold: separate groups, feedback sections configured, and the score table or chart enabled.
myreport.php lets students review their own responses and, when feedback sections exist, compare their scores with the average for their group. In separate-groups mode it only overrides the group when it is 0, and it never checks membership.
A student in Group A of a separate-groups questionnaire with feedback scores shown opens:
/mod/questionnaire/myreport.php?instance=<qid>&action=vresp&group=<GroupB id>
The score table header shows Group B's name and its average section percentages. Any group id from another course also has its name shown.
$currentgroupid = optional_param('group', 0, PARAM_INT); // Groupid.
Validate the group right after the existing capability check, as report.php does:
if (!$questionnaire->capabilities()->can_read_own_responses() || $userid != $USER->id ||
!$questionnaire->capabilities()->can_view_group((int)$currentgroupid)) {
throw new \moodle_exception('nopermissions', 'mod_questionnaire');
}
$currentgroupresps = $questionnaire->get_responses(false, $currentgroupid);
$rawgroupname = groups_get_group_name($currentgroupid);
In questionnaire::get_recent_mod_activity(), the separate-groups branch indexes $modinfo->groups by $cm->id. Core keys that array by grouping id, so the lookup is almost always undefined, and passing null to array_intersect() throws a TypeError on PHP 8. The recent-activity report therefore crashes for any grader without moodle/site:accessallgroups in a separate-groups questionnaire with other users' attempts. Separately, $usersgroups is computed once for the first attempt's user and reused for every later attempt, so the filter would compare the wrong user's groups even if the lookup worked. Core quiz recomputes the groups for each attempt and uses $modinfo->get_groups($cm->groupingid).
Low risk. Currently a fatal error, so it fails closed rather than leaking data, but the report is unusable for affected teachers.
The function feeds the course 'recent activity' report. The faulty branch only runs for graders who lack accessallgroups in separate-groups mode, typically non-editing teachers.
if ($usersgroups === null) {
$usersgroups = groups_get_all_groups(
$course->id,
$attempt->userid,
$cm->groupingid
);
if (is_array($usersgroups)) {
$usersgroups = array_keys($usersgroups);
} else {
$usersgroups = [];
}
}
if (!array_intersect($usersgroups, $modinfo->groups[$cm->id])) {
continue;
}
$usersgroups = array_keys(groups_get_all_groups($course->id, $attempt->userid, $cm->groupingid));
if (!array_intersect($usersgroups, $modinfo->get_groups($cm->groupingid))) {
continue;
}
The backup structure step has three gaps:
- It omits the
progressbarcolumn ofquestionnaire, so the setting silently resets to off on backup, restore or duplicate. - It never annotates the survey's
feedbacknotesfile area, even though restore callsadd_related_files('mod_questionnaire', 'feedbacknotes', ...). Images embedded in feedback notes are lost. - When a questionnaire uses another course's public survey, the survey, questions and all responses are excluded, and restore keeps the backup's raw
sid. On another site that id points to an unrelated survey. A crafted backup could also point it deliberately at any survey id, which would show that survey's questions in the restoring teacher's course.
Low risk. The main impact is silent data loss on duplicate or restore. The crafted-sid case needs restore and backup-upload capabilities and only exposes question text.
These are the standard Moodle 2 backup and restore steps, used by course backup, import and activity duplication.
'autonum',
'removeafter',
]);
Add 'progressbar', to the questionnaire element's field list.
$survey->annotate_files('mod_questionnaire', 'thankbody', 'id'); // By survey->id.
$survey->annotate_files('mod_questionnaire', 'feedbacknotes', 'id');
$newitemid = $DB->insert_record('questionnaire', $data);
When no survey element is restored, check that $data->sid exists and is a public survey on the target site. Otherwise, reset it or fail the restore with a clear message.
submit_questionnaire_response::execute() calls self::validate_parameters() but ignores the cleaned array it returns. It then passes the original arguments, including the unvalidated $action and $responses, to save_mobile_data(). The sibling question_reorder external correctly unpacks the validated values. The current use is harmless because $action is only compared with string literals and the scalar arguments are typed int. Even so, any later reliance on cleaned values would silently be bypassed.
Low risk. This is a coding-convention gap with no current exploit path.
This is the mobile-app submission web service. Authorisation (validate_context, require_capability, the response-ownership check and user_access_messages) is correct.
self::validate_parameters(
self::execute_parameters(),
[
$params = self::validate_parameters(self::execute_parameters(), [...]);
['cmid' => $cmid, 'sec' => $sec, 'rid' => $rid, 'action' => $action, 'responses' => $responses] = $params;
The 2018050102 upgrade step runs UPDATE {questionnaire_response} qr INNER JOIN {questionnaire_attempts} qa ... SET .... That multi-table UPDATE syntax is MySQL/MariaDB-only and fails on PostgreSQL, MSSQL and Oracle. A non-MySQL site upgrading from a pre-2018 questionnaire release would hit a fatal upgrade error at this step.
Low risk. It only affects long-dormant non-MySQL installations, but there it blocks the upgrade.
This is a one-off data migration from the old attempts table, run only for sites whose installed plugin version is older than 2018050102.
$sql = 'UPDATE {questionnaire_response} qr ' .
'INNER JOIN {questionnaire_attempts} qa ON qr.id = qa.rid ' .
'SET qr.questionnaireid = qa.qid';
$DB->execute($sql, []);
Use a portable correlated subquery:
$sql = 'UPDATE {questionnaire_response}
SET questionnaireid = (SELECT qa.qid FROM {questionnaire_attempts} qa
WHERE qa.rid = {questionnaire_response}.id)
WHERE EXISTS (SELECT 1 FROM {questionnaire_attempts} qa
WHERE qa.rid = {questionnaire_response}.id)';
The "Other" text inputs in the app templates use a literal placeholder="Enter: ". The PDF factory sets the document author and title to the literals 'Moodle Questionnaire' and 'All responses'. These strings cannot be translated, unlike the rest of the plugin, which uses get_string() throughout. The mobile templates already receive translated strings via the lang list in db/mobile.php and the translate pipe, so the same mechanism can be used.
Low risk. This is a localisation defect only.
These are user-visible strings in the Moodle app response form and in the metadata of PDF exports.
<ion-input placeholder="Enter: " [(ngModel)]="CONTENT_OTHERDATA.<%otherchoicekey%>"
Add a language string, list it in db/mobile.php, and use {{ 'plugin.mod_questionnaire.enterother' | translate }}. Apply the same change to latest/checkbox_question.mustache and both ionic5 templates.
$pdf->SetAuthor('Moodle Questionnaire');
$pdf->SetTitle('All responses');
$pdf->SetAuthor(get_string('modulename', 'questionnaire'));
$pdf->SetTitle(get_string('allresponses', 'questionnaire'));
| Library | Version | License | Declared |
|---|---|---|---|
RGraph Canvas charts (bipolar, hbar, radar, rose, vprogress) for feedback score charts, loaded on demand by the mod_questionnaire/chart AMD module | 2013 (unversioned build) | MIT | ✓ |
Trusted-text migration. The 2026113100.03 upgrade step (questionnaire_upgrade_trust_existing_content()) marks all existing staff-authored HTML as trusted. That preserves the previous unfiltered rendering for legacy content. It only has an effect where the site has trusted text enabled (enabletrusttext), and all newly saved content is flagged correctly through trusttext_trusted(). Administrators who want legacy content cleaned can reset the *trust columns to 0.
External email reporting. When an administrator enables questionnaire/allowemailreporting, teachers can send each submission, including the respondent's name and email address for named questionnaires, to arbitrary external addresses. They can also email full exports to any address from the download page. This is intended and admin-gated, but sites should treat that setting as a data-export control.
Bundled RGraph build dates from 2013. It is declared in thirdpartylibs.xml, and the plugin only uses it to draw canvas text and shapes from a JSON spec in a data attribute, with no HTML tooltips. Consider replacing it with core core/chartjs charts in the long term.
Public-survey questionnaires omit responses from backups. When a questionnaire uses another course's public survey, the backup step skips the response tables as well as the survey definition, so student responses for that activity are not preserved by course backups.