Guess It
qtype_guessit
qtype_guessit is a Moodle question type that presents word- or sentence-guessing games similar to Wordle. The teacher enters a target phrase or single uppercase word; the renderer displays one input gap per word (or per letter, in Wordle mode) and provides per-attempt feedback with coloured markup for correct, partially correct, misplaced, and incorrect characters. The plugin depends on the companion qbehaviour_guessit behaviour to drive the multi-try grading flow. It ships with backup/restore, an XML import/export round-trip, a null_provider privacy class, English language strings, and AMD modules that handle gap-to-gap keyboard navigation.
The plugin is structurally sound: it uses the Moodle $DB API with parameterised queries, delegates pluginfile serving to core's question_pluginfile(), ships a privacy provider (null), defines its own DB table cleanly in db/install.xml, and follows the standard question-type architecture. The main concern is the specific-feedback rendering path, which embeds previously submitted student responses into HTML without escaping. Because student gap data is collected with PARAM_RAW_TRIMMED and never sanitised before concatenation, an attacker can inject a script payload through a gap response. Practical impact is limited: the vulnerable branch only runs while the question state is todo (between Check presses, before submission), so the dominant scenario is self-XSS — a student attacking themselves — rather than cross-user reach. Remaining findings are code-quality items: a remote Google Fonts @import in the stylesheet, a $plugin->supported range that does not include Moodle 5.0, two interface methods (is_complete_response, get_validation_error) that return null instead of the documented types, an unusual htmlspecialchars_decode() of stored responses, dead parameters in update_question_guessit(), a JS scoping bug in wordlenavigation.js when multiple wordle questions render on the same page, and a couple of minor coding-style issues (uppercase <?PHP and GLOBAL).
qtype_guessit is a clean, focused question type with a single notable security concern and several low-severity code-quality items.
Security
- Stored XSS in
qtype_guessit_renderer::specific_feedback(): previously submitted gap responses are concatenated directly into HTML output. Any student can inject<script>(or other HTML) through a normal gap submission, and the payload renders the next time the same attempt page is shown. The state guard ($qa->get_state() != question_state::$todo) keeps the vulnerable branch from running once the attempt is finalised, so review-time exposure to teachers is unlikely under normal workflows; the realistic impact is self-XSS during an in-progress attempt.
Code quality / compliance
styles.cssperforms@import url('https://fonts.googleapis.com/...'), sending every learner's IP and Referer to a third party at page render — a privacy/data-protection concern for installations that need to control external requests.version.phpdeclares$plugin->supported = [401, 405], which omits Moodle 5.0 (branch500). Since the review target is Moodle 5.0, the plugin should explicitly include it.is_complete_response()andget_validation_error()inquestion.phpreturnnull(a barereturn;). Both are part of thequestion_manually_gradableinterface and core behaviours call them directly.embedded_element()callshtmlspecialchars_decode()on the stored response before re-using it. The decoding is unnecessary (responses are stored verbatim underPARAM_RAW_TRIMMED) and obscures what data the renderer is actually working with.update_question_guessit()accepts$optionsand$contextparameters that are immediately overwritten or unused.amd/src/wordlenavigation.jsqueriesinput.correctat the document level inside a per-question loop; with multiple Wordle questions on one page, thefinishedflag is computed against gaps from other questions.- Minor coding-style violations:
<?PHP(uppercase) inversion.phpandGLOBAL $CFG;(uppercase) inquestion.php.
Architecture
- File serving via
qtype_guessit_pluginfile()correctly delegates toquestion_pluginfile(). - Backup/restore, XML import/export, the install schema, and the
null_providerprivacy class look correct. - Behat coverage exists for create/preview/grade, backup/restore and import/export. There are no PHPUnit tests; only a
helper.phpthat defines fixtures used by Behat.
Findings
The renderer's specific_feedback() iterates over the student's previous attempt steps and concatenates each submitted gap value into HTML output without escaping.
Gap responses are collected with PARAM_RAW_TRIMMED (see qtype_guessit_question::get_expected_data() in question.php), which performs no HTML cleaning. A student can therefore submit <script>alert(1)</script> (or any HTML) as the value of any gap, and on the next render of the same question the payload is emitted verbatim inside a <div>.
The rendered string is then passed straight through html_writer::nonempty_tag('div', $this->specific_feedback($qa), ...) in the feedback() method, which wraps the HTML but does not escape it.
The vulnerable branch is gated by $qa->get_state() == question_state::$todo and "not yet fully correct". In the adaptive-style flow used by qbehaviour_guessit, that branch is exactly what runs between Check presses during an in-progress attempt — so the malicious payload renders inside the same student's session immediately after they submit it. After the attempt is submitted/graded the state is no longer todo and the branch returns early, which is what limits the practical blast radius.
Medium risk. The XSS is reachable via normal gameplay — no privileged role is required to inject the payload — but the early return in specific_feedback() ensures the rendering branch does not run once the attempt state moves past todo. In practice this means:
- Self-XSS during the attempt is reliable. A student who deliberately injects a payload will execute it in their own browser between Check presses.
- Teacher review of finished attempts is not affected because the question state at review time is one of the graded/final states (
gradedwrong,gradedpartial,complete,gaveup, etc.), which makes$issubmittedtrue and the function returns empty. - Cross-user reach is limited to the unusual case where a privileged user (admin, teacher with override) opens an in-progress attempt that still has
todostate for the affected question.
The code defect is unambiguous and the fix is a one-line change to wrap each concatenated response with s().
get_expected_data() declares each gap field as PARAM_RAW_TRIMMED, so the engine stores responses with no HTML cleaning. get_all_responses() then walks the question_attempt step iterator and collects raw $step->get_qt_data() arrays. In the regular (non-Wordle) branch of specific_feedback() each response value is concatenated into HTML; in the Wordle branch each character is concatenated. The result is wrapped by the parent feedback() via html_writer::nonempty_tag('div', ..., ['class' => 'specificfeedback']), which preserves the inner HTML.
For comparison, embedded_element() is safe in this respect because the response value is placed into an input element's value attribute through html_writer::empty_tag(), which performs attribute escaping.
- Open a quiz containing a guessit question.
- In the first gap, enter:
<img src=x onerror=alert(document.cookie)>
…and fill the other gaps with arbitrary text so the attempt is partially incorrect.
3. Press Check. The adaptive-style behaviour returns the question state to todo, the renderer calls specific_feedback(), and the previously submitted answers — including the unescaped <img> tag — are emitted into the page. The onerror handler fires.
$formattedfeedback .= '<div class="specific-feedback input-wrapper '.$colorclass.'">'.
$studentanswer. '<span class="feedback-markup">'.$markupcode. '</span></div>';
Escape the response before concatenating into HTML. s() (alias of htmlspecialchars) is the standard helper in Moodle for plain text:
$formattedfeedback .= '<div class="specific-feedback input-wrapper '.$colorclass.'">'.
s($studentanswer).
'<span class="feedback-markup">'.$markupcode.'</span></div>';
Also review the htmlspecialchars_decode() call in embedded_element() (line 166) — it should not be necessary if responses are escaped consistently when they reach the renderer.
$formattedfeedback .= '<div class="specific-feedback input-wrapper '.$colorclass.'">'.
$studentletters[$index]. '</div>';
Although the JavaScript in Wordle mode restricts gap input to single A–Z letters, that is a client-side control only. A crafted POST can still place arbitrary characters into per-letter responses. Escape the value:
$formattedfeedback .= '<div class="specific-feedback input-wrapper '.$colorclass.'">'.
s($studentletters[$index]).'</div>';
styles.css opens with an @import that pulls font CSS from fonts.googleapis.com on every page that includes the plugin's styles.
This sends the learner's IP address, User-Agent, and Referer (which discloses the Moodle URL the user is on) to Google on every page render. For installations that need to control external requests for privacy/GDPR reasons, this is a regression because Moodle core does not load fonts from fonts.googleapis.com. It also creates a runtime dependency on an external service: if the Google CDN is blocked or unreachable (corporate networks, sites behind strict CSPs, the Chinese internet, etc.) the stylesheet's downstream rules still apply but the chosen font face silently falls back.
Low risk. Not a security vulnerability per se. The concern is privacy/compliance (third-party data sharing without explicit opt-in, which is sensitive under GDPR-style regulations) and availability (external CDN is a single point of failure outside the operator's control).
The stylesheet is automatically loaded by Moodle whenever the plugin is rendered (every page that displays a guessit question). The @import happens client-side, so this is not a server-side outbound HTTP call — but it does cause every learner's browser to contact Google.
@import url('https://fonts.googleapis.com/css2?family=Roboto+Mono&family=Roboto:wght@300&display=swap');
Either bundle the font files locally inside pix/ or a dedicated fonts/ subdirectory and reference them through a local @font-face declaration, or fall back to system fonts. The existing fallback chain already lists 'Lucida Console', Courier, monospace, which would work without any external download.
version.php declares $plugin->supported = [401, 405], i.e. Moodle 4.1 through 4.5. Moodle 5.0 corresponds to branch 500, which falls outside this range. Because $plugin->requires is enforced (4.1+) but $plugin->supported is informational, installation does not fail — but the plugin advertises that it has not been validated against the version it is being installed on.
Low risk. Pure metadata mismatch — the plugin still installs and runs because requires is satisfied. The administrator-facing warning during install/upgrade is the only practical effect.
Moodle 5.0's version.php reports $branch = '500'. Plugins maintain $plugin->supported as a hint to admins and to the plugins directory, telling them which Moodle versions the maintainer has actually tested.
$plugin->supported = [401, 405];
Update the upper bound to include the Moodle 5.x branches that have been tested, e.g. [401, 501]. Also consider extending the .github/workflows/moodle-ci.yml matrix to add MOODLE_500_STABLE so that the supported claim is backed by CI runs.
Two methods that are part of the question_manually_gradable / question_with_responses contract return a bare return; (i.e. null):
is_complete_response()is documented to returnbooland is consulted by behaviours (e.g.qbehaviour_adaptive::process_submit()) when deciding whether the response is complete enough to grade. Returningnullis loosely falsy, so the question always looks incomplete to those callers.get_validation_error()is documented to return astringfor display when the question state is invalid.
The inline comment on is_complete_response() says it has been replaced by check_complete_answer() in the renderer. That replacement is only used inside the renderer; core behaviours still call the question class methods. The companion qbehaviour_guessit may compensate for this, but the qtype contract is still being violated.
Low risk. Functional behaviour appears to work because qbehaviour_guessit and the renderer maintain their own state, but the type-contract violation is fragile under future Moodle versions that may add stricter typing.
Both methods are part of the abstract question_manually_gradable interface (see /moodle/question/type/questionbase.php) and are called by core behaviour classes during attempt processing. PHP's loose typing hides the type mismatch but the documented contract is not honoured.
public function is_complete_response(array $response) {
return;
}
Return an explicit bool. A natural implementation is the same logic the renderer uses: every gap in $response must be a non-empty string.
public function is_complete_response(array $response): bool {
foreach (array_keys($this->places) as $place) {
if (!array_key_exists($this->field($place), $response)
|| trim((string) $response[$this->field($place)]) === '') {
return false;
}
}
return true;
}
public function get_validation_error(array $response) {
return; // Not used by guessit.
}
Return an empty string (or a real validation message) so callers that concatenate the result into HTML do not silently coerce null:
public function get_validation_error(array $response): string {
return '';
}
embedded_element() decodes HTML entities on the stored student response before using it. Responses are written to the question_attempts step data with PARAM_RAW_TRIMMED, which performs no encoding, so the decode is not undoing any earlier escape — it only affects the rare case where a response actually contains literal < etc. (e.g. a learner who typed entity references).
The practical effect is that <script> typed by a student becomes <script> before any downstream processing. In the input field's value attribute the browser still receives an attribute-encoded string thanks to html_writer, but anywhere this decoded value is concatenated into HTML without escaping it would now contain raw < and >. Combined with the unescaped concatenation in specific_feedback() (finding 1), this slightly broadens the payload surface.
Low risk. Standalone the decode is benign, but it is a code smell that masks the actual data-flow assumptions. Removing it improves clarity and reduces the post-fix surface for finding 1.
get_last_qt_var() returns whatever was stored in the question_attempts step data. Since get_expected_data() declares the type as PARAM_RAW_TRIMMED, the engine does not encode the value at submit time — meaning there is nothing to decode at render time.
$studentanswer = $qa->get_last_qt_var($fieldname) ?? '';
$studentanswer = htmlspecialchars_decode($studentanswer);
Drop the decode. If a response did contain < it should be displayed as <, not decoded back to <:
$studentanswer = $qa->get_last_qt_var($fieldname) ?? '';
If the original intent was to handle a backup/restore artefact, narrow the call to that exact code path with a comment explaining why.
amd/src/wordlenavigation.js iterates each [id^="question-"] element on the page (per-question), but inside that loop it queries input.correct against the whole document rather than the current question scope. The finished flag is then computed by comparing the document-wide count of .correct inputs against the current question's gap count. With more than one Wordle question on the same page the count is incorrect: a question that is not yet finished can be classified as finished (or vice versa) depending on the state of the other questions.
Low risk. Functional/UX bug only — the worst case is that input fields are made pointer-events: none; when they should remain editable, or the cursor style is wrong. No security impact.
Quizzes can render multiple questions per page (and several can use the guessit question type). The document-wide selector mixes data across them.
document.querySelectorAll('[id^="question-"]').forEach(question => {
const gaps = question.querySelectorAll('input[type="text"][name*="p"][class*="wordlegap"]');
const correctGaps = document.querySelectorAll('input.correct');
const finished = (correctGaps.length == gaps.length);
Scope the query to the current question:
const correctGaps = question.querySelectorAll('input.correct.wordlegap');
Also remember to rebuild amd/build/wordlenavigation.min.js (and its source map).
update_question_guessit($question, $options, $context) declares three parameters but immediately overwrites $options from the database (so the passed value is ignored) and never references $context at all. The dead parameters are also passed by the caller, where the $context is computed for no reason.
Low risk. Maintenance/readability only.
save_question_options() calls this method right after fetching the options from the DB, so by the time the helper runs the caller already has the current record — yet the helper re-fetches it.
public function update_question_guessit($question, $options, $context) {
global $DB;
$options = $DB->get_record('question_guessit', ['question' => $question->id]);
if (!$options) {
$options = new stdClass();
...
Drop the unused parameters and the corresponding caller arguments:
public function update_question_guessit($question) {
global $DB;
$options = $DB->get_record('question_guessit', ['question' => $question->id]);
...
}
version.php uses <?PHP (uppercase open tag) and question.php::make_behaviour() uses GLOBAL $CFG; (uppercase keyword). Moodle's coding style requires <?php and global; these forms also trip Moodle Code Checker.
Low risk. Cosmetic only.
Both files follow Moodle conventions otherwise; these are isolated case-sensitivity slips.
public function make_behaviour(question_attempt $qa, $preferredbehaviour) {
GLOBAL $CFG;
public function make_behaviour(question_attempt $qa, $preferredbehaviour) {
global $CFG;
The tests/ directory contains a helper.php (test fixtures) and three Behat features (basic flow, Wordle flow, backup/restore, import/export) but no PHPUnit unit tests. Question types typically benefit from PHPUnit coverage of grading, response handling, and import/export round-trips. The CI workflow runs moodle-plugin-ci phpunit --fail-on-warning, so PHPUnit tests would be exercised automatically if added.
Informational. Existing Behat coverage is solid; adding PHPUnit tests for grading and the markup helper would harden the plugin further.
Behat covers the user-facing flow but is slower and does not catch many internal regressions (e.g. the is_complete_response() return-type issue noted above, or the markup-builder edge cases in get_markup_string()).
File serving is correctly delegated. qtype_guessit_pluginfile() requires questionlib.php and forwards to core's question_pluginfile(), which performs the necessary access checks (login, edit-tab capability for exports, qubaid validation). No additional capability check is required in the plugin.
Database access is portable. All DB calls go through $DB with parameterised conditions; the schema in db/install.xml is well-formed; the plugin's own question_guessit table is the only one it writes to (apart from the standard question_answers core table that question types are expected to manage). No raw SQL strings are concatenated.
The Privacy provider is a null_provider with a language-string reason (privacy:null_reason). Given that the question type stores no personal data of its own — student responses are stored by the question engine, which has its own privacy provider — this is the correct choice.
make_behaviour() forces the guessit behaviour when the companion plugin is installed. The plugin's version.php declares qbehaviour_guessit as a hard dependency, so the fall-through to make_archetypal_behaviour() is effectively unreachable in normal installs. This is a deliberate design choice rather than a defect, but worth noting because the runtime feedback flow depends on qbehaviour_guessit's state-transition logic.
Form validation for Wordle mode in edit_guessit_form.php enforces uppercase A–Z and a max length of 8 server-side, which is good. Note that the byte-level strlen() is used; for ASCII-only inputs this is fine, but if the validation is ever extended to allow accented characters the check should switch to mb_strlen().