STACK MathQuill Editor
local_stackmatheditor
local_stackmatheditor ("STACK MathQuill Editor") is a Moodle local plugin that overlays a visual MathQuill formula editor, with a configurable symbol toolbar, onto STACK (qtype_stack) question input fields in mod_quiz and mod_adaptivequiz.
It injects its JavaScript runtime through Moodle output hooks (before_standard_top_of_body_html_generation, before_footer_html_generation), converts between MathQuill LaTeX and Maxima CAS notation client-side (tex2max/max2tex), and stores per-quiz / per-question toolbar configuration in its own table. Teachers configure the toolbar via configure.php (a moodleform page gated by mod/quiz:manage / mod/adaptivequiz:viewreport); a read-only web service exposes the resolved toolbar config for AJAX use. It depends on qtype_stack and bundles MathQuill 0.10.1 as a declared third-party library.
The plugin demonstrates strong security fundamentals throughout. Every database query uses parameterised $DB methods (no string concatenation of variables into SQL); the server-to-client JSON hand-off is encoded defensively (JSON_HEX_TAG plus a second json_encode() and assignment to textContent, immune to </script> breakout); the JS modules build the DOM with .text()/.attr() and reserve .html() for hard-coded, server-side toolbar definitions rather than any user input; the configuration page uses a moodleform (automatic sesskey/CSRF handling) behind require_login() and require_capability(); and the return-URL flow is hardened against open redirects via PARAM_LOCALURL and an explicit resolve_return_url() allow-list (with unit tests proving javascript:, protocol-relative and backslash-trick URLs are rejected). Test coverage (PHPUnit, Behat, Jest, Playwright) is unusually thorough.
No critical, high, or medium issues were found. The three low-severity findings are: (1) a debug helper that calls error_log() unconditionally on essentially every page request site-wide despite documentation claiming it is gated on developer debug mode (operational/log-hygiene, no data impact); (2) configure.php resolving and type-checking the requested question via two DB queries — and throwing distinct exceptions — before require_login()/require_capability(), allowing an unauthenticated visitor to trigger those queries and distinguish generic error pages for a guessed course-module and question (defense-in-depth; the only data exposed is whether a question is a STACK question, and all functionality and stored configuration remain fully gated); and (3) an incomplete Privacy API implementation that declares a stored usermodified field but provides no export or deletion request provider. None enable harm to other users' data, so the ceiling is low, and the otherwise clean, well-tested codebase keeps it at the top of that band.
Overview
local_stackmatheditor is a well-engineered plugin that adds a visual MathQuill formula editor to STACK question inputs. The review covered all shipped PHP (entry points, classes/, db/), all 13 AMD JavaScript modules, the language pack, and the shipped PHPUnit/Behat tests. Development-only trees (tools/, .github/, docs/, tests/{load,playwright,jest}) are export-ignored and do not ship to production.
Security posture
The plugin follows Moodle security conventions carefully:
- SQL — every query in
config_managerandquiz_helperuses named placeholders and$DB->get_in_or_equal(); no variable is concatenated into a query string. - Output encoding —
page_helper::inject_json_element()encodes payloads withJSON_HEX_TAG | JSON_UNESCAPED_UNICODE | JSON_THROW_ON_ERROR, double-encodes into a JS string literal, and writes toscript.textContent, which is robust against</script>breakout and HTML injection. The consuming JS uses.text()/.attr()/MathQuill's LaTeX parser for dynamic values; the only.html()calls intoolbar.jsoperate exclusively on hard-coded toolbar definitions fromdefinitions.php, never on stored config or user input. - Access control —
configure.phpenforcesrequire_login()and the module-appropriate manage capability; the externalget_configservice isloginrequiredand callsvalidate_context(), and returns only non-sensitive toolbar-visibility data. - CSRF / redirects — configuration is saved through a
moodleform(automatic sesskey); thereturnurlparameter is constrained byPARAM_LOCALURLand a dedicated allow-list validator with unit tests covering malicious inputs.
Findings
No critical/high/medium issues. Three low-severity findings and one informational note:
- Low (code quality) —
quiz_helper::dbg()writes to the PHP error log on every request site-wide, contradicting its own docblock. - Low (security) —
configure.phpperforms question resolution/validation before authentication, giving an unauthenticated visitor a trivial pre-auth probe. - Low (compliance) — the Privacy API declares stored user data but implements no export/erasure provider.
- Info — dead scaffolding variables in
db/upgrade.php.
Third-party code
MathQuill 0.10.1 (MPL-2.0) is bundled and correctly declared in thirdpartylibs.xml with an accompanying readme_moodle.txt.
Findings
quiz_helper::dbg() calls PHP's error_log() unconditionally. Its docblock states the message is "Only emitted when Moodle developer debug mode is active ($CFG->debug >= DEBUG_DEVELOPER). Silent on production sites.", but the implementation performs no debug-level check at all.
Crucially, the before_footer hook calls dbg() before the page-type gate (if (!$iseditor && !$isconfigure) { return; }). Because the hook only bails out early when the plugin is hard-disabled (enabled mode 0, which is not the default), a log line such as [SME-HOOK] before_footer: page=my-index editor=N configure=N is written for essentially every page rendered anywhere on the site. Further dbg() calls fire on quiz edit/attempt/review, question preview and adaptive-quiz pages.
Consequences:
- The web server error log is filled with one or more entries per request across the whole site, which obscures genuine PHP warnings/errors and consumes disk over time.
- The misleading docblock makes it likely this behaviour ships to production unnoticed.
error_log()is on Moodle's forbidden-functions list (the calls carry aphpcs:ignore), so the intent was clearly developer-only tracing.
Low risk. This is an operational / log-hygiene defect, not a security vulnerability: no user data is exposed to end users, and the logged strings (page type, cmid, error text) are only visible to whoever can read the server error log. The blast radius is nonetheless site-wide — one or more log lines per request — so on a busy site it can bury real errors and grow the log unboundedly. It is the most impactful of the low findings and the contradiction with its own docblock makes it easy to miss.
hook_callbacks::before_footer() is registered against \core\hook\output\before_footer_html_generation (see db/hooks.php), which fires on the render of every Moodle page. The only early return before the dbg() call is plugin_could_be_active() (enabled mode != 0), so the log write is effectively unconditional site-wide.
public static function dbg(string $msg): void {
// phpcs:ignore moodle.PHP.ForbiddenFunctions.FoundWithAlternative
error_log('[SME-HOOK] ' . $msg);
}
Gate the log write on the developer debug level so the behaviour matches the documentation and stays silent in production:
public static function dbg(string $msg): void {
if (!debugging('', DEBUG_DEVELOPER)) {
return;
}
// phpcs:ignore moodle.PHP.ForbiddenFunctions.FoundWithAlternative
error_log('[SME-HOOK] ' . $msg);
}
Also remove the duplicated/contradictory docblock above the method.
quiz_helper::dbg(
'before_footer: page=' . $PAGE->pagetype
. ' editor=' . ($iseditor ? 'Y' : 'N')
. ' configure=' . ($isconfigure ? 'Y' : 'N')
);
if (!$iseditor && !$isconfigure) {
return;
}
Independently of the fix in dbg(), this trace should sit after the if (!$iseditor && !$isconfigure) { return; } gate so that non-quiz pages produce no logging work at all.
In configure.php, require_login() and require_capability() are only reached at lines 101 and 106. Before them, when a qbeid/questionid is supplied (question mode), the script resolves the question bank entry (config_manager::resolve_qbeid()), runs a second query against {question}/{question_versions}, and throws distinct exceptions depending on the result: cannotresolveqbeid when nothing matches and notstackquestion when the question exists but is not a STACK question.
Because all of this runs before authentication, an unauthenticated visitor who supplies a valid cmid together with a qbeid/questionid triggers those queries and can tell the outcomes apart from the resulting error pages, whereas the success path only then falls through to require_login() (a redirect to the login page).
Loading the course module and course before require_login() is the standard Moodle idiom (their results are arguments to it), so that part is fine. The question-resolution block is what runs too early and should be moved below the capability check.
Low risk. require_login() and require_capability() are enforced before any configuration is displayed, saved, or otherwise acted upon, so there is no unauthorised access to functionality or to stored configuration. The only pre-authentication exposure is two DB queries and the ability to distinguish a couple of generic error messages for a guessed cmid + question — at most revealing whether a given question is a STACK question. No user data or configuration is disclosed. This is a defense-in-depth ordering issue: authenticate first, then do question-specific work.
configure.php is the teacher-facing toolbar configuration page, normally opened from a quiz edit/attempt page. Its authorisation gate (require_login() + require_capability('mod/quiz:manage' | 'mod/adaptivequiz:viewreport')) sits below the parameter parsing and the question-resolution logic. cmid values are sequential and not secret, so a valid one is easy to supply.
Without a Moodle session, request /local/stackmatheditor/configure.php?cmid=<valid_quiz_cmid>&questionid=<id>. Depending on whether <id> resolves and whether it is a STACK question, the response is either a cannotresolveqbeid/notstackquestion error page (rendered before authentication) or a redirect to the login page — a distinguishable difference that reveals question-type metadata and executes two DB queries pre-authentication.
// Question resolution (mod_quiz question-mode only).
$questionrecord = null;
if (!$quizmode) {
if ($qbeid <= 0 && $questionid > 0) {
$qbeid = config_manager::resolve_qbeid($questionid);
}
if (!$qbeid) {
throw new \moodle_exception('cannotresolveqbeid', 'local_stackmatheditor');
}
$questionsql = "
SELECT q.id, q.name, q.qtype, qv.version
FROM {question} q
JOIN {question_versions} qv ON qv.questionid = q.id
WHERE qv.questionbankentryid = :qbeid
ORDER BY qv.version DESC";
$questionversions = $DB->get_records_sql($questionsql, ['qbeid' => $qbeid], 0, 1);
$questionrecord = $questionversions ? reset($questionversions) : null;
if (!$questionrecord) {
throw new \moodle_exception('cannotresolveqbeid', 'local_stackmatheditor');
}
if ($questionrecord->qtype !== 'stack') {
throw new \moodle_exception('notstackquestion', 'local_stackmatheditor');
}
$questionid = (int) $questionrecord->id;
}
// Context and permissions.
$context = \context_module::instance($cmid);
require_login($course, false, $cm);
Establish the context and run the authentication/authorisation checks immediately after the course module and course are resolved, then perform question resolution:
$course = get_course($cm->course);
$context = \context_module::instance($cmid);
require_login($course, false, $cm);
$capname = $isadaptivequiz ? 'mod/adaptivequiz:viewreport' : 'mod/quiz:manage';
require_capability($capname, $context);
// ... only now resolve/validate the requested question ...
The plugin's table local_stackmatheditor stores usermodified — the ID of the user who created or last changed each toolbar-configuration record (written in config_manager::upsert_record() from $USER->id). classes/privacy/provider.php correctly declares this through add_database_table(), mapping usermodified to privacy:metadata:usermodified ("The person who last modified the configuration").
However, the provider implements only \core_privacy\local\metadata\provider. It does not implement \core_privacy\local\request\plugin\provider or \core_privacy\local\request\core_userlist_provider. As a result, the personal data the plugin itself acknowledges storing cannot be discovered, exported, or deleted in response to a data-subject (GDPR) request. A metadata-only provider is appropriate only for plugins that store no personal data locally (or only forward it to a described external/subsystem location).
Low risk. The affected data is limited to configuration audit metadata (which user last edited a toolbar configuration, and when) rather than sensitive personal content, and the records are created only by teachers/managers. This is a GDPR completeness gap, not a data exposure: the practical impact is that subject access and erasure requests will silently omit this plugin's records.
Records are created when a teacher/manager saves a per-quiz or per-question toolbar configuration; usermodified records the acting user. The plugin even ships a unit test (tests/unit/language_strings_test.php::test_privacy_metadata_strings_and_fields) that confirms the declared privacy fields exist as real columns — so the storage of user-linked data is acknowledged and verified, but the request side is absent.
class provider implements metadata_provider {
Implement the request interfaces so the declared data can be serviced. At minimum:
use core_privacy\local\request\core_userlist_provider;
use core_privacy\local\request\plugin\provider as request_provider;
class provider implements
\core_privacy\local\metadata\provider,
request_provider,
core_userlist_provider {
// get_contexts_for_userid(), get_users_in_context(),
// export_user_data(), delete_data_for_all_users_in_context(),
// delete_data_for_user(), delete_data_for_users()
// keyed on the usermodified field / the module context.
}
If the audit field is genuinely considered out of scope, that decision should be justified — but since it is declared as personal data, the request provider is the expected implementation.
The upgrade entry point xmldb_local_stackmatheditor_upgrade() assigns $dbman = $DB->get_manager(); and $targettable = new xmldb_table('local_stackmatheditor'); but never uses either before return true;. This is leftover scaffolding from a template. It is harmless (and $DB->get_manager() is legitimate inside db/upgrade.php), but the dead assignments should be removed so future upgrade steps start from a clean function.
Informational. No functional, performance, or security impact.
The plugin's schema has not changed since install, so the upgrade function is a no-op; the unused variables are the remains of scaffolding.
function xmldb_local_stackmatheditor_upgrade(int $oldversion): bool {
global $DB;
$dbman = $DB->get_manager();
$targettable = new xmldb_table('local_stackmatheditor');
return true;
}
function xmldb_local_stackmatheditor_upgrade(int $oldversion): bool {
// No upgrade steps yet.
return true;
}
| Library | Version | License | Declared |
|---|---|---|---|
MathQuill Client-side WYSIWYG mathematics editor that renders the visual formula input fields the plugin overlays on STACK question inputs. Loaded from `thirdparty/mathquill/` by `editor_injector`/`mathquill_init`. | 0.10.1 | MPL-2.0 | ✓ |
Output encoding is well designed. page_helper::inject_json_element() encodes payloads with JSON_HEX_TAG | JSON_UNESCAPED_UNICODE | JSON_THROW_ON_ERROR, double-encodes into a JS string literal, and assigns to script.textContent — robust against </script> breakout and HTML injection. The external service get_config and editor_injector reuse the same JSON_HEX_TAG discipline. No change needed; noted as a positive.
Toolbar .html() usage is safe in context. toolbar.js inserts el.display_html, el.display_latex, and backslash-containing el.label via jQuery .html(). These values come exclusively from the hard-coded definitions::get_element_groups() server-side constants (the sole display_html is a literal fraction glyph); the teacher-controlled slot config only toggles which hard-coded groups render, never the element markup. There is therefore no user-input path into .html(), and no XSS.
Minor client-side inefficiency in mathjax_shim.install(). The setInterval loop keeps calling ensureHub() every 20 ms for up to ~10 s and re-creates the MathJax.Hub façade object on each tick even after a real MathJax v3 is present, because the post-install state no longer matches its early-exit conditions. It is idempotent and harmless, but the interval could be cleared once a genuine MathJax is detected to avoid the repeated work on quiz attempt pages.
Cosmetic: wrong @package tag in two developer tools. tools/mustache_check.php and tools/fix_phpdoc.php carry @package local_coursectrl (copied from another plugin) instead of local_stackmatheditor. Both are phpcs:ignoreFile and the whole tools/ directory is export-ignored, so this never ships and never breaks CI, but the tags should be corrected.