HTML5 canvas sketch game
mod_gcanvas
mod_gcanvas ("Canvas game") is a Moodle activity module that lets students create freehand HTML5 canvas sketches using the bundled Fabric.js library. Students can add shapes, text, emoji, a teacher-supplied background and toolbar images, upload their own images, then save, download, restore and delete their drawings ("attempts"). Teachers configure a background, toolbar shapes and an editable help text. The module integrates AJAX endpoints, the Moodle File API, backup/restore and a full Privacy (GDPR) provider.
The codebase demonstrates generally sound security practices. Every state-changing entry point validates the session (require_sesskey()), authenticates the user against the specific course module (require_login($course, true, $cm)), and all privileged file operations are gated by require_capability(). All database access is parameterised, the File API is used correctly (no direct filesystem, DB-driver, HTTP or shell access), user-supplied help text is rendered through format_text() with cleaning enabled, mustache output is auto-escaped, and a complete Privacy API implementation is present. The plugin was recently hardened (release 4.5.1) and I verified those fixes are in place — the previously-reported stored XSS in the help text and the IDOR on attempt images are both correctly remediated.
What remains are low-severity gaps, with no high or critical issues and no medium findings. The most notable is that the attempt file area received an ownership check during the recent hardening but the sibling student_image area did not — it is still served to any participant holding mod/gcanvas:view. Practical impact is limited because those filenames are never exposed to other users, so retrieval depends on guessing the filename. The save_canvas AJAX endpoint also writes the posted base64 payload to a .png file with no size or content-type validation and no guest check, enabling minor storage abuse (mitigated by per-user scoping and forced-download serving). The remainder are code-quality and robustness defects: the backup annotates help-text files with the wrong itemid, the restore step silently drops student attempts, the teacher capability omits the RISK_XSS flag, gcanvas_add_instance() writes a non-existent column, and the plugin ships no automated tests. The pattern — correct security fundamentals with a few residual access-control and validation gaps — places the plugin just below the top tier.
Overview
mod_gcanvas is a student-facing HTML5 canvas drawing activity. The review covered all 24 PHP files, the AMD JavaScript source (amd/src/canvas.js), all four mustache templates, the database/access definitions, backup/restore, the privacy provider, and project metadata. Third-party libraries (Fabric.js, Spectrum, Twemoji) are correctly declared in thirdpartylibs.xml and were not subject to Moodle-rule review.
Security posture
The plugin's security fundamentals are solid:
- Authentication & session:
ajax.phpperformsrequire_login($course, true, $cm)followed byrequire_sesskey()before dispatching any action;view.phpandindex.phplikewise callrequire_login(). The custom upload handler inview.phpis gated byconfirm_sesskey(). - Authorization: privileged file uploads are funnelled through
helper::upload_file(), which enforcesmod/gcanvas:teacher(background / toolbar) ormod/gcanvas:student_image(student uploads).gcanvas_pluginfile()checksmod/gcanvas:viewand additionally restricts theattemptarea to its owner or a teacher. - Injection: all
$DBcalls use parameterised conditions or named placeholders; no string-built SQL. No direct database driver, filesystem, HTTP, or shell access anywhere in the plugin. - Output encoding: the only unescaped (
{{{ }}}) template values areformat_text()output (purified) and renderedmoodleformHTML. User-controlled data reaches templates only through escaped{{ }}tags. - Per-user scoping: history, get/delete-attempt and the privacy queries are all filtered by
$USER->id, so there is no cross-user data leakage through the AJAX layer. - Privacy: a complete provider implements metadata, export, and all three deletion paths.
I confirmed that the fixes described in the 4.5.1 changelog (LS-4198) are genuinely present: the help-text editor now requires mod/gcanvas:teacher and is rendered with cleaning enabled, and the attempt file area enforces ownership.
Findings
No critical, high, or medium issues were found. Seven low-severity findings are reported:
- The
student_imagefile area lacks the ownership check applied toattempt, so it is readable by any participant who can guess the filename. - The
save_canvasendpoint stores unvalidated, unbounded data with no size/type check or guest block. - Backup mis-annotates help-text files (itemid
idvs. the stored itemid0), excluding embedded images from backups. - Restore never processes
gcanvas_attemptrecords, silently dropping attempts on restore-with-user-data. - The
mod/gcanvas:teachercapability omitsRISK_XSSdespite gating an HTML editor. gcanvas_add_instance()writes a non-existenttimecreatedcolumn (dead code / schema drift).- No automated tests; CI disables both PHPUnit and Behat.
The overall picture is a well-structured, recently-hardened plugin with correct security fundamentals and a small set of residual low-risk gaps.
Findings
gcanvas_pluginfile() only enforces the mod/gcanvas:view capability for most file areas and adds an owner/teacher restriction exclusively for the attempt area. The student_image area — which holds images individual students upload from their own device to place on their canvas — is stored under an item id equal to the uploader's user id but is not restricted to that owner.
As a result, any participant holding mod/gcanvas:view (every enrolled user, since the capability is granted to the user archetype) can fetch another user's uploaded image by requesting its pluginfile.php URL, provided they know the file name.
This is the same class of issue (an IDOR on a personal file area) that the 4.5.1 release fixed for the attempt area, but the sibling student_image area was not given the same treatment.
Low risk. The vulnerable path is reachable by any enrolled user, but practical exploitation is constrained by the fact that student_image file names are never disclosed to anyone other than the uploader, so an attacker must guess the exact file name (the context and item id are discoverable). The exposed content is a drawing-aid image the student chose to upload, which is generally low-sensitivity. There is no write or modification capability here — read-only disclosure. The blast radius is limited to other participants of the same activity. Nonetheless this is a genuine access-control gap of the same kind that was remediated for the attempt area, so it should be closed for consistency and defence in depth.
student_image files are uploaded through helper::upload_file() (which stores them under itemid = $USER->id) and are returned to the uploader's own canvas by callable_upload_images(). Crucially, helper::get_images('student_image', ...) is only ever called inside upload_file() to echo the just-uploaded image back to its owner — these file names are never listed to other users anywhere in the UI or AJAX responses. The shared areas background and toolbar_shape, by contrast, are deliberately listed to all participants via get_images() in the renderer and callable_get_toolbar_images().
As student B (co-enrolled in the same course/module as victim student A), request:
https://moodle.example/pluginfile.php/<modulecontextid>/mod_gcanvas/student_image/<studentA_userid>/<filename>
The mod/gcanvas:view check passes and send_stored_file() returns student A's uploaded image. The user id is enumerable; only the original file name (e.g. a common value such as image.png) must be known or guessed.
$itemid = (int) array_shift($args);
// Attempts are personal: only the owner (or a teacher) may retrieve them (LS-4198).
if ($filearea === 'attempt') {
$attempt = $DB->get_record('gcanvas_attempt', ['id' => $itemid], 'id, user_id, gcanvas_id', IGNORE_MISSING);
if (!$attempt) {
return false;
}
if ($attempt->user_id != $USER->id && !has_capability('mod/gcanvas:teacher', $context)) {
return false;
}
}
Extend the ownership gate to cover the personal student_image area. Because that area is keyed by the uploader's user id (helper::upload_file() sets $itemid = $USER->id), the owner can be derived directly from the item id:
if ($filearea === 'student_image') {
if ((int) $itemid !== (int) $USER->id && !has_capability('mod/gcanvas:teacher', $context)) {
return false;
}
}
Keep background and toolbar_shape readable by all participants — those are intentionally shared, teacher-provided resources.
callable_save_canvas() decodes the client-supplied canvas_data (a base64 data URI) and writes the raw bytes to a stored file named time() . '.png' via create_file_from_string(). Unlike the file-picker upload path — which enforces accepted_types => 'web_image' and maxbytes through helper::get_file_options() — this path performs no size limit and no content-type validation: the actual bytes are never checked to be an image, yet they are stored under a .png name.
The method also performs no capability check and no isguestuser() guard, relying solely on the require_login() + require_sesskey() performed by the router. Any user who can view the module (and a guest, if guest access is enabled on the course) can therefore create unlimited gcanvas_attempt rows and their associated files.
Low risk. Content-type spoofing is effectively neutralised: attempt files are served as forced downloads (so a disguised HTML/SVG payload is not rendered inline in the Moodle origin) and are only retrievable by the owner or a teacher, so this is not a viable stored-XSS vector. The residual concern is resource abuse — unbounded, repeated writes by an authenticated user (or a guest where guest access is enabled) consuming database rows and file storage. This requires deliberate abuse, is bounded per request by PHP upload limits, and affects only server capacity rather than other users' data, hence low severity. Adding a guest guard, a size cap and an image-validity check closes the gap cheaply.
The endpoint is the intended mechanism for students to persist their drawings, so write access for ordinary participants is by design and the records are scoped to $USER->id. The concern is the absence of the validation that the parallel file-picker path applies. Stored attempt files are served by gcanvas_pluginfile() with send_stored_file($file, 0, 0, true, ...) — i.e. $forcedownload = true — and only to the owner or a teacher, and the history view only renders files that pass is_valid_image().
Authenticated POST to /mod/gcanvas/ajax.php with a valid sesskey, action=save_canvas, and data={"id":<cmid>,"status":"final","canvas_data":"data:image/png;base64,<very large blob>","json_data":"{}"}. Each request stores one file sized up to the PHP post limit; scripting many requests inflates filedir storage and the gcanvas_attempt table. Replacing the blob with non-image bytes stores arbitrary content under a .png name.
public function callable_save_canvas(): array {
global $DB, $USER;
$fileid = 0;
$cobject = $this->load_cm_and_course();
$imagecontent = base64_decode(preg_replace('#^data:image/\w+;base64,#i', '', $this->data->canvas_data));
if (!empty($imagecontent)) {
$attemptid = $DB->insert_record('gcanvas_attempt', (object) [
'status' => $this->data->status,
'user_id' => $USER->id,
'gcanvas_id' => $cobject->cm->instance,
'json_data' => $this->data->json_data,
'added_on' => time(),
]);
// Create the image.
$modulecontext = context_module::instance($cobject->cm->id);
$fs = new file_storage();
$fileid = $fs->create_file_from_string((object) [
'contextid' => $modulecontext->id,
'component' => 'mod_gcanvas',
'filearea' => 'attempt',
'itemid' => $attemptid,
'filepath' => '/',
'filename' => time() . '.png',
'userid' => $USER->id,
], $imagecontent);
}
Add input hardening to the endpoint:
- Reject guests:
if (isguestuser()) { return ['success' => false]; }(or require an explicit submit capability). - Bound the payload size before decoding, e.g. compare
strlen($imagecontent)againstget_max_upload_file_size()/$CFG->maxbytesand abort if exceeded. - Validate the content is really an image, e.g. via
$fs->create_file_from_string()followed byis_valid_image()(deleting it if invalid), or by checking the decoded bytes withgetimagesizefromstring(). - Validate
statusagainst an allow-list (e.g.final,draft) instead of storing the raw value into thechar(10)column.
The help-text file area is saved with item id 0 in view.php (file_save_draft_area_files(..., 'helptext', 0, ...)), matching how the helptext editor is prepared. However, the backup step annotates the helptext files with the field name 'id', i.e. it tells the backup engine the files live under itemid = gcanvas.id (a non-zero value).
Because no helptext files actually exist under that item id, images embedded in the activity help text are silently excluded from backups (and therefore lost on restore/duplicate). The neighbouring intro area is correctly annotated with null (the item-id-0 convention).
Low risk. This is a data-fidelity/backup-correctness defect, not a security issue. Its only effect is that pictures inserted into the help-text editor are dropped from backups and course duplicates; the textual help content itself (a DB column) is preserved. No user data exposure or integrity risk to live data.
In view.php the intro action saves help-text content and any embedded files to file_save_draft_area_files($draftitemid, $modulecontext->id, 'mod_gcanvas', 'helptext', 0, ...). The item id is hard-coded to 0. The backup define_structure() then annotates the same area with 'id', which resolves to the gcanvas record id at backup time — a value that never matches the stored files.
$gcanvas->annotate_files('mod_gcanvas', 'intro', null); // This file areas haven't itemid.
$gcanvas->annotate_files('mod_gcanvas', 'helptext', 'id'); // This file areas haven't itemid.
$gcanvas->annotate_files('mod_gcanvas', 'toolbar_shape', 'id'); // This file areas haven't itemid.
$gcanvas->annotate_files('mod_gcanvas', 'background', 'id'); // This file areas haven't itemid.
Annotate the helptext area with null so it is backed up using the item-id-0 convention, consistent with how view.php stores it:
$gcanvas->annotate_files('mod_gcanvas', 'helptext', null);
(background and toolbar_shape are correctly annotated with 'id', since helper::upload_file() stores them under itemid = $PAGE->cm->instance, which equals gcanvas.id.)
The backup step defines a nested gcanvas_attempt element and, when user info is included, sets its source table and annotates the attempt files and the user_id mapping. The restore step, however, defines only a path element for /activity/gcanvas — there is no restore_path_element for gcanvas_attempt and no process_gcanvas_attempt() method.
Consequently, student attempts (and the add_related_files('mod_gcanvas', 'attempt', 'gcanvas_attempt') call in after_execute(), which references a gcanvas_attempt mapping that is never created) are silently discarded on restore, even when the administrator chose to include user data. The backup and restore halves are therefore inconsistent.
Low risk. No security impact. The consequence is data loss on restore/duplicate when user data is included: students' saved sketches and their images are not recreated. For a non-graded drawing activity this is a robustness/quality defect rather than a critical data-integrity failure, but it contradicts the backup definition and should be fixed so backups round-trip correctly.
backup_gcanvas_stepslib.php builds attempts -> gcanvas_attempt children and, under if ($userinfo), sets the source table, annotates attempt files and annotate_ids('user', 'user_id'). The restore class only inserts the gcanvas record and calls apply_activity_instance(); nothing consumes the attempt sub-tree.
protected function define_structure(): array {
$paths = [];
$paths[] = new restore_path_element('gcanvas', '/activity/gcanvas');
// Return the paths wrapped into standard activity structure.
return $this->prepare_activity_structure($paths);
}
Add a path element and processor for attempts (mirroring the backup structure), and map the new id so after_execute() can restore the related files:
$paths[] = new restore_path_element('gcanvas', '/activity/gcanvas');
$paths[] = new restore_path_element('gcanvas_attempt',
'/activity/gcanvas/attempts/gcanvas_attempt');
protected function process_gcanvas_attempt($data): void {
global $DB;
$data = (object) $data;
$oldid = $data->id;
$data->gcanvas_id = $this->get_new_parentid('gcanvas');
$data->user_id = $this->get_mappingid('user', $data->user_id);
$newid = $DB->insert_record('gcanvas_attempt', $data);
$this->set_mapping('gcanvas_attempt', $oldid, $newid, true); // last arg = files exist.
}
$this->add_related_files('mod_gcanvas', 'attempt', 'gcanvas_attempt');
Once a gcanvas_attempt mapping is created by a process_gcanvas_attempt() method (see above), this call will correctly relocate the attempt images. Without that mapping it is a no-op.
mod/gcanvas:teacher is the capability that gates the help-text editor (the intro action in view.php), which stores raw HTML (PARAM_RAW, trusttext => true). Moodle convention is to declare 'riskbitmask' => RISK_XSS on any capability that allows a user to store HTML/script that other users will view, so that administrators are warned of the risk when assigning the capability. This flag is present on mod/gcanvas:addinstance but missing on mod/gcanvas:teacher.
Low risk. This is an advisory metadata/best-practice gap rather than an exploitable vulnerability — the output is purified, so injected scripts do not execute. Declaring RISK_XSS is nonetheless the correct convention: it surfaces the risk to administrators reviewing role assignments and documents that the capability permits HTML authoring. No runtime impact.
The help text is captured by the intro form (PARAM_RAW) and rendered by output_canvas::export_for_template() through format_text($intro, FORMAT_HTML, ['noclean' => false, ...]). Because cleaning is enabled, the HTML purifier strips scripts on output, so a stored-XSS exploit is not actually achievable via this path.
'mod/gcanvas:teacher' => [
'captype' => 'write',
'contextlevel' => CONTEXT_MODULE,
'archetypes' => [
'teacher' => CAP_ALLOW,
'editingteacher' => CAP_ALLOW,
],
],
Declare the XSS risk on the capability that allows storing HTML:
'mod/gcanvas:teacher' => [
'riskbitmask' => RISK_XSS,
'captype' => 'write',
'contextlevel' => CONTEXT_MODULE,
'archetypes' => [
'teacher' => CAP_ALLOW,
'editingteacher' => CAP_ALLOW,
],
],
gcanvas_add_instance() sets $moduleinstance->timecreated = time(); before inserting, but the gcanvas table defined in db/install.xml has no timecreated column (it has timemodified only). Moodle's insert_record() silently strips object properties that do not correspond to a column, so the assignment is harmless dead code — but it indicates schema drift and is misleading (a reader assumes a creation timestamp is persisted when it is not).
Low risk. No functional or security impact — the value is dropped by the DML layer and instance creation works. This is purely a code-cleanliness / schema-consistency issue.
Confirmed against core: mysqli_native_moodle_database::insert_record() builds its column list from get_columns($table) and skips any field where !isset($columns[$field]), so the stray timecreated value is discarded without error.
$moduleinstance->timecreated = time();
$id = $DB->insert_record('gcanvas', $moduleinstance);
Either remove the unused assignment, or add the column to db/install.xml (and a db/upgrade.php step) if a creation timestamp is actually wanted:
<FIELD NAME="timecreated" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
Note that gcanvas_update_instance() correctly sets timemodified, which does exist.
The plugin ships no tests/ directory, and the CI workflow explicitly disables both PHPUnit and Behat runs. For a plugin that handles file uploads, AJAX state changes, capability checks and backup/restore — and that has just undergone a security patch — the absence of any automated regression coverage increases the chance that future changes silently reintroduce a vulnerability or break the File API / backup contracts.
Low risk. This is a maintainability and assurance gap, not a direct vulnerability. Its significance is amplified by the security-sensitive surface (file serving, AJAX writes, capability gates) and the recent security release, where regression tests would guard the just-applied fixes.
The repository uses the shared catalyst/catalyst-moodle-workflows CI, which provides PHPUnit/Behat stages, but both are turned off via the with: inputs. No tests/ fixtures or test classes exist in the plugin.
with:
disable_behat: true
disable_phpunit: true
Add at least minimal automated coverage and re-enable the corresponding CI stages:
- A PHPUnit test for
gcanvas_pluginfile()access control (owner vs. non-owner vs. teacher forattemptandstudent_image),lib.phpCRUD, and the privacy provider export/delete paths. - A Behat feature covering create-activity, save-sketch and teacher help-text editing.
Then set disable_phpunit: false / disable_behat: false (or remove the overrides).
| Library | Version | License | Declared |
|---|---|---|---|
Fabric.js HTML5 canvas drawing/serialization engine that powers the entire sketch surface (shapes, text, images, undo, toDataURL/loadFromJSON). | 2.4.2 | MIT | ✓ |
Spectrum jQuery colour-picker used to set the fill colour of selected canvas shapes. | 1.8.0 | MIT | ✓ |
spectrum.css Stylesheet for the Spectrum colour picker, loaded via $PAGE->requires->css() in view.php. | 1.8.0 | MIT | ✓ |
twitter/twemoji Emoji PNG/SVG graphics shown in the emoji picker and placed onto the canvas. | 11.2 | CC-BY 4.0 | ✓ |
Recent security hardening verified. Release 4.5.1 (changelog entry LS-4198) claims to fix a stored XSS in the help text and an IDOR on attempt images. I confirmed both fixes are present in the reviewed code: the intro action in view.php now calls require_capability('mod/gcanvas:teacher', ...), output_canvas renders the help text with format_text(..., ['noclean' => false]), and gcanvas_pluginfile() restricts the attempt area to its owner or a teacher. The per-user student_image item id, the restore decode rules, and the attempt-row delete cascade in gcanvas_delete_instance() are likewise in place.
Version metadata vs. documentation. version.php declares $plugin->supported = [39, 500] (up to Moodle 5.0) and $plugin->requires = 2020061500 (Moodle 3.9), while README.md states a maximum of Moodle 4.5.x. This is a harmless documentation inconsistency, but aligning the README with supported would avoid confusion.
Minor style: file storage accessor. classes/ajax.php instantiates new file_storage() directly in callable_save_canvas(), whereas the rest of the plugin uses the conventional get_file_storage() singleton accessor. Both work, but using get_file_storage() consistently is preferable.
Client-side HTML assembly. amd/src/canvas.js builds the toolbar-image list by string-concatenating src values into markup (selectToolbarImage()). The values originate from moodle_url::make_pluginfile_url()->out() (trusted, server-generated URLs), so there is no injection risk today; using DOM/$('<img>') construction would be more robust if the data source ever changes.
db/uninstall.php not required. The plugin stores data only in its own tables (gcanvas, gcanvas_attempt) and standard module file areas, all of which Moodle cleans up automatically on uninstall, so the absence of db/uninstall.php is correct here.