E-Language Learning: Video Dictation Activity
mod_elang
mod_elang (eLang) is a language-learning activity module. Learners fill gaps in a timed transcript that is synchronised to an audio/video medium — an uploaded file, a direct media URL, or a curated YouTube/Vimeo embed. The plugin provides a React-based authoring editor (subtitle import, gap placement, rule-based gap generation, hints, accepted-answer variants), an attempt lifecycle driven entirely through AJAX external functions, two grading algorithms (exact and word-recognised with configurable Jaro similarity and pluggable per-script normalisation), a teacher attempt report with dataformat export, transcript worksheet/solution export to PDF/DOCX/ODT/TXT, gradebook and custom-completion integration, course backup/restore, course reset support, a complete Privacy (GDPR) API, and a resumable one-way migration path from mod_elang 1.x.
The plugin demonstrates an unusually strong and consistent security posture across a large surface (7 web entry points, 13 external functions, backup/restore, a migration subsystem, exports and a client-side player).
No security vulnerabilities were found. The controls that matter are all present and applied consistently:
- Every page and external function pairs
require_login/validate_contextwith an appropriaterequire_capability, and attempt endpoints add an explicit ownership check (require_attempt_ownership) so a learner cannot reach another learner's attempt by guessing an id. - Solutions never leak to learners:
get_attempt_cuesruns every transcript throughtranscript_masker::mask()and omitssolution/answer variants; the transcript export gates the unmasked solution behind a capability and an activity setting; thepluginfilecallback enforces per-version file access so draft media cannot be fetched by URL guessing. - SQL is uniformly parameterised, with a strict whitelist for the report's
ORDER BY. Output is escaped by Mustache{{ }}(triple-brace only for framework HTML) and the player builds DOM withcreateTextNode/createElement, so there is no XSS or SQLi path. There is no server-side URL fetching (so no SSRF), no raw DB access, no shell execution, and no directdatarootaccess. - Author-supplied regular expressions are gated behind a separate
mod/elang:useregexcapability, run with a control-character delimiter against length-capped input. - A complete Privacy API, CSRF protection (
confirm_sesskeyon the manual delete/migration actions,moodleformelsewhere), locking + transactions around all state changes, and DoS limits on the subtitle/import parsers round out a defensive design.
The only findings are three low-severity, code-quality items: a missing MOODLE_INTERNAL guard on lib.php (which has a file-scope define()), a redundant manual require_once plus an unneeded guard in one external class, and the V1 decommissioner performing table/column drops through $DB->get_manager() outside db/upgrade.php. The last is admin-only, explicitly confirmed and deliberately designed, so it carries no exploit path; its only real consequence is a schema drift from install.xml. None of the three affect security, and none is reachable by a lower-privileged user, which places the plugin at the top of the code-quality-only band rather than any security band.
Overview
mod_elang 4.5 is a large, mature language-exercise activity (RC1 of a 2.0 rewrite). I read every PHP, JavaScript/TypeScript and Mustache file in full and cross-checked the framework APIs it relies on against Moodle core at /moodle.
Security assessment: strong
The plugin is defensively engineered throughout, and I could not construct any exploit path:
- Access control — Every entry point (
view,edit,media,report,transcript,index,admin_migrate_v1) and every external function enforcesrequire_login/validate_contextplus the right capability. Attempt endpoints additionally verify$attempt->userid === $USER->id, so a student cannot touch another student's attempt. - Solution confidentiality — Transcripts sent to the player are gap-masked (
transcript_masker); solutions/answer variants are never serialised into learner-facing payloads; the solution export is gated by capability and the per-activitysolutionavailabilitysetting;mod_elang_pluginfileenforces per-version file access (draft media is never served to non-managers). - Injection — All DB access is parameterised; the report's sort column is whitelisted. Output is escaped (Mustache
{{ }}; player DOM viacreateTextNode); the DOCX/ODT writers XML-escape all text. React components use JSX escaping only (nodangerouslySetInnerHTMLin plugin code). - No dangerous sinks — No
curl/file_get_contents(URL)/sockets (no SSRF), no rawmysqli/PDO, noeval/exec, no superglobals, nodatarootwrites. Temp files usemake_request_directory(); all persistent files use the File API. - Robustness — Author regexes are capability-gated (
mod/elang:useregex), control-char delimited and length-capped; the subtitle/import parsers enforce size, line-length and cue-count limits; all state changes run under Moodle locks + delegated transactions. - Compliance — A complete Privacy API (metadata incl. the video-provider external link, contexts, userlist, export, and all delete variants), full backup/restore with id remapping, and course reset support.
Findings (all low, code quality)
lib.phpperforms a file-scopedefine()without thedefined('MOODLE_INTERNAL') || die();guard — amoodle.Files.MoodleInternalviolation that will fail the maintainer's phpcs CI.classes/external/generate_rule_gaps.phpmanuallyrequire_onces a trait that Moodle already autoloads and carries a guard its siblings do not need.v1_decommissionerdrops legacy tables and theelang.optionscolumn via$DB->get_manager()outsidedb/upgrade.php; admin-only and deliberate, but it leaves the live schema inconsistent withinstall.xml.
Grade: A
No security, compliance-blocking, or data-integrity issues; only minor style/architecture nits. The three low findings keep it from A+.
Findings
lib.php changes global state at file scope — it calls define('ELANG_GRADE_REBUILD_CHUNK', 500); — but does not start with the defined('MOODLE_INTERNAL') || die(); guard, and it does not include config.php itself.
Moodle's coding standard, enforced by the official moodle-cs linter that moodle-plugin-ci runs in CI, requires the guard in exactly this situation: a file that executes file-scope side effects and does not require config.php must declare the guard (sniff moodle.Files.MoodleInternal, message MoodleInternalGlobalState).
This is the standard convention for module library files. Core modules follow it — for example mod/folder/lib.php and mod/label/lib.php both place the guard immediately above their file-scope define() calls.
This is a coding-standard defect only; it has no security or runtime impact because lib.php is always included by core after config.php has loaded. Its practical effect is that a phpcs run against the Moodle standard will report an error and break the maintainer's CI.
Low risk. No security impact and no runtime effect. The concrete consequence is a failing phpcs/moodle-plugin-ci gate and a deviation from the documented file convention that every other guard-carrying file in this plugin (and core) follows.
lib.php holds the standard module callbacks (elang_supports, elang_add_instance, gradebook and completion callbacks, mod_elang_pluginfile, reset/export helpers). It is loaded by core, which always has MOODLE_INTERNAL defined, so the guard is a static-analysis/convention requirement rather than a runtime one.
define('ELANG_GRADE_REBUILD_CHUNK', 500);
Add the guard immediately after the file docblock, before the define() (matching the core convention shown in mod/label/lib.php):
defined('MOODLE_INTERNAL') || die();
/**
* How many learners a whole-activity grade rebuild processes per batch, ...
*/
define('ELANG_GRADE_REBUILD_CHUNK', 500);
classes/external/generate_rule_gaps.php is the only external-function class that carries a defined('MOODLE_INTERNAL') || die(); guard and manually require_onces the authoring_helper trait at file scope. Its sibling external classes (save_draft_version.php, publish_version.php, set_draft_media.php, preview_import.php, get_version_content.php, and the attempt classes) do neither.
The require_once is redundant: \mod_elang\external\authoring_helper is a namespaced trait under classes/, so core_component autoloads it automatically the moment the class that uses it is defined. The manual include therefore adds a needless file-scope side effect to an autoloaded class file.
Because that require_once is the file's only side effect, once it is removed the MOODLE_INTERNAL guard also becomes unnecessary — moodle-cs warns on a guard in a side-effect-free single-class file (moodle.Files.MoodleInternal.MoodleInternalNotNeeded). So both lines should go together.
The file does not currently crash: because plugin classes are registered in the core_component classmap, its autoload path runs with global $CFG in scope, so $CFG->dirroot resolves. The issue is purely redundant, non-idiomatic code, not a functional fault.
Low risk. No security or functional impact. It is a maintenance/consistency nit that also risks a MoodleInternalNotNeeded warning from the linter once the redundant include is noticed.
The class is an AJAX external function (mod/elang:manage-gated) that returns gap spans for the authoring editor. It uses the shared authoring_helper trait exactly the way its siblings do; only this file includes the trait explicitly and guards the file.
defined('MOODLE_INTERNAL') || die();
require_once($CFG->dirroot . '/mod/elang/classes/external/authoring_helper.php');
Remove both lines. The use authoring_helper; inside the class body is sufficient — Moodle autoloads the trait — and the class then matches every other file under classes/external/:
namespace mod_elang\external;
use core_external\external_api;
// ... remaining use statements ...
/**
* Generate gap definitions from a rule for the authoring editor.
* ...
*/
class generate_rule_gaps extends external_api {
use authoring_helper;
v1_decommissioner::decommission() obtains $DB->get_manager() and drops the legacy tables elang_cues, elang_users, elang_help, elang_check and the elang.options column outside of db/upgrade.php.
Moodle's convention is that all schema changes flow through db/upgrade.php, tied to a version bump and executed under the controlled upgrade path. The author documents a genuine reason for not doing so here: an unconditional destructive DROP in upgrade.php would delete a site's only remaining copy of its V1 source data automatically on upgrade, whether or not the migration was ever verified. The action is therefore admin-triggered, guarded by blockers(), and requires explicit confirmation.
Even granting that rationale, one concrete defect remains: elang.options is still declared in install.xml (and is listed as a backed-up field in backup/moodle2/backup_elang_stepslib.php). Dropping it at runtime leaves the live database schema inconsistent with install.xml on any site that has completed the migrate → sign-off → decommission cycle. Moodle's admin "Check database schema" health report will then flag elang.options as a missing column, and the backup/restore field list references a column that no longer exists.
This is not a security issue: the only callers are admin_migrate_v1.php (which requires moodle/site:config via admin_externalpage_setup) and cli/decommission_v1.php (CLI). A site administrator can already perform arbitrary DDL, so there is no privilege escalation and no path reachable by a lower-privileged user.
Low risk. Not exploitable — the operation is administrator-only, explicitly confirmed, and gated by blockers(); a site admin can already alter the schema directly, so there is no privilege boundary crossed. The tangible consequence is limited to sites that have completed the full V1 decommission: their live schema no longer matches install.xml for the elang.options column, which the database-schema health check reports and which the backup field list still references. It is a Moodle architecture/coding-guideline deviation with a contained, recoverable operational impact rather than a security or data-loss risk to end users.
decommission() is invoked only from the site-admin migration page (admin_migrate_v1.php, moodle/site:config + confirm_sesskey) and the CLI script. It runs only when blockers() is empty (every V1 activity migrated and signed off) and, for elang.options, only when at least one activity was actually signed off. The class docblock explains the deliberate choice to keep this out of db/upgrade.php.
$dbman = $DB->get_manager();
$result = (object) ['droppedtables' => [], 'droppedfields' => []];
foreach (['elang_cues', 'elang_users', 'elang_help', 'elang_check'] as $name) {
$table = new \xmldb_table($name);
if ($dbman->table_exists($table)) {
$dbman->drop_table($table);
$result->droppedtables[] = $name;
}
}
Two independent points:
1. Keep the schema in step with install.xml. Because elang.options is dropped by this code, install.xml should not continue to declare it on the same release — otherwise fresh installs and decommissioned upgrades diverge. Options: drop options from install.xml (and from the backup field list) in the same release that first ships the decommission action, or defer the elang.options removal to a real, admin-flag-gated db/upgrade.php step so the declared schema and the live schema always agree.
2. If the runtime drop is kept, document clearly (as is already partly done) that it is an intentional, admin-only, one-way operation, and consider surfacing the resulting schema state to the admin (the health-check will otherwise report options as unexpectedly absent). The legacy-table drops themselves are lower concern since those tables are not part of this plugin's declared schema.
$elangtable = new \xmldb_table('elang');
$optionsfield = new \xmldb_field('options', XMLDB_TYPE_TEXT);
if ($dbman->field_exists($elangtable, $optionsfield)) {
$dbman->drop_field($elangtable, $optionsfield);
$result->droppedfields[] = 'elang.options';
}
See the guidance on the first location. The elang.options drop is the part that creates the install.xml inconsistency; align the declared schema with the runtime behaviour, or move this column removal into a controlled db/upgrade.php step.
| Library | Version | License | Declared |
|---|---|---|---|
React Runtime for the bundled authoring editor (js/vendor/react/editor.bundle.js), loaded on Moodle versions (4.5-5.1) that do not ship React in core. | 18.3.1 | MIT | ✓ |
ReactDOM DOM renderer for React, bundled into the authoring editor bundle. | 18.3.1 | MIT | ✓ |
Scheduler ReactDOM runtime dependency, bundled into the authoring editor bundle. | 0.23.2 | MIT | ✓ |
Overall this is an exemplary, security-conscious codebase. Access control, object-level ownership checks, solution masking, parameterised SQL with a sort whitelist, escaped output, capability-gated author regexes, a complete Privacy API, full backup/restore, and lock+transaction discipline around every state change are all present and consistent. Several defensive touches go beyond the norm — e.g. the mod_elang_pluginfile callback documents and fixes a prior draft-media disclosure, and get_attempt_cues deliberately omits gap charstart/charlength so the masked payload cannot leak solution length as a free hint.
Bundled React will overlap core on Moodle 5.2+. The plugin ships its own React/ReactDOM bundle because Moodle 4.5-5.1 have no React runtime. From Moodle 5.2 core ships React, so on those versions the plugin carries a second copy. This is not a live problem — the bundle is loaded as a standalone page script exposing window.mod_elang_editor rather than as an AMD module named react, so there is no module-name collision — and the plugin already documents the plan (in thirdpartylibs.xml and edit.php) to switch to core's React once the minimum supported version rises to 5.2. Noted here only as a forward-looking maintenance item, not a finding.
db/subplugins.json declares both subplugintypes and plugintypes. This is a deliberate cross-version-compatibility choice (the newer plugintypes key alongside the legacy one) for the elangscript subplugin type and is harmless; no action needed.
Client-side provider embeds are handled with privacy in mind. YouTube/Vimeo frames are embedded only from the curated provider_registry, with encodeURIComponent'd canonical ids, referrerpolicy="strict-origin", and an optional per-session consent gate driven by the site providerconsent setting; the transfer of IP/user-agent to the provider is also declared in the Privacy API. No provider URL is ever fetched server-side, so there is no SSRF surface.