🕹️PlayerHUD Filter
filter_playerhud
PlayerHUD text filter — a companion to the block_playerhud gamification block. It expands three shortcodes ([PLAYERHUD_WIDGET], [PLAYERHUD_DROP code=...], [PLAYERHUD_TRADE code=...]) embedded in course and activity content into an interactive HUD widget, item-collection cards and inline trade offers. Rendering is delegated to dedicated output classes (text_filter, output\render, output\widget, output\assets) and Mustache templates. The plugin stores no data of its own; it reads the block's gamification tables and renders links back to the block's own collect.php / process_trade.php / view.php endpoints.
This is a well-engineered, security-conscious filter with no security vulnerabilities and only minor, well-mitigated code-quality observations.
Security controls are thorough and correct:
- Every database query uses
$DBwith bound placeholders (orget_in_or_equal); no string concatenation of user input into SQL. - The only user-influenced shortcode inputs (drop/trade
code) are constrained to[a-zA-Z0-9]+by regex and passed as parameters regardless. - All rendered output is escaped: user/teacher-authored item and class descriptions are cleaned with
format_text()before reaching the client, names go throughformat_string(), hand-built data attributes are individually wrapped ins(), and templates rely on Mustache auto-escaping. Triple-mustache is used only for values that are already sanitised (format_text()output or core-generated HTML). - The filter gates rendering behind
isloggedin()/isguestuser()/SITEIDchecks, an explicitblock/playerhud:viewcapability check on the block context, and a per-user gamification-enabled check — stripping shortcodes cleanly when any check fails. - Block
configdatais decoded withunserialize_object(), preventing PHP object injection. - A deliberate
'filter' => falseonformat_text()prevents unbounded filter re-entrancy.
No direct filesystem, raw DB, outbound HTTP, shell, or schema-modification access exists. The Privacy API is correctly implemented as a null_provider (the filter stores nothing). The plugin ships extensive PHPUnit and Behat coverage, including explicit regression tests for the XSS, capability, recursion and object-injection hardening.
The residual observations are architectural: the filter is tightly coupled to block_playerhud's internal database schema (mitigated by a declared hard dependency and defensive table_exists / field_exists / method_exists guards), and it assembles data-* attributes as a raw HTML string emitted through unescaped triple-mustache (currently fully escaped, but a fragile pattern). Neither is exploitable. The overall standard — parameterised queries, consistent escaping, layered access control, and matching tests — is high.
filter_playerhud is a companion text filter for the block_playerhud gamification block. It converts three shortcodes into an interactive HUD widget, drop-collection cards, and trade offers.
Review outcome: clean. I read every PHP, template, test and CSS file in the shipped plugin and found no security vulnerabilities.
Strengths observed:
- Access control in depth — the filter checks login/guest/site-course state, enforces
block/playerhud:viewon the block context, and honours the per-user gamification toggle before rendering anything. Unauthorised paths strip the shortcodes entirely. - Output encoding — item/class descriptions are run through
format_text()(verified against the modal-injection path), names throughformat_string(), and every dynamic value placed into hand-built attribute strings is wrapped ins(). Templates use auto-escaping; the few triple-mustache sinks carry only pre-sanitised or core-generated HTML. - Safe data handling — all queries are parameterised;
unserialize_object()blocks object injection fromconfigdata; a'filter' => falseguard prevents filter recursion;nulldescriptions are handled. - Test coverage — PHPUnit and Behat suites include targeted regression tests for description XSS, the capability gate, shortcode-recursion, object injection and null descriptions.
Minor observations (non-security): the filter reads block_playerhud's tables directly (tight schema coupling, mitigated by a declared dependency and *_exists guards), and it builds data-* attributes as a raw string injected via {{{ }}} (safe today, but fragile). No third-party libraries are shipped (the docs/ site assets are export-ignored and excluded from the release archive), so no thirdpartylibs.xml is required.
Findings
The filter issues raw SQL and record lookups directly against tables owned by the block_playerhud plugin rather than going through that plugin's public API. Affected tables include block_playerhud_drops, block_playerhud_items, block_playerhud_inventory, block_playerhud_stack, block_playerhud_stack_log, block_playerhud_trades, block_playerhud_trade_reqs, block_playerhud_trade_rewards, block_playerhud_user, block_playerhud_rpg_progress and block_playerhud_classes.
The plugin also depends on specific columns of those tables (for example block_playerhud_drops.value, block_playerhud_stack_log.delta / source). If the block plugin renames or restructures any of these, the filter breaks at runtime rather than at install time.
This is a maintainability / robustness concern, not a security one:
- All queries are parameterised — there is no injection exposure.
- The plugin declares a hard dependency on
block_playerhud(version.php), so the tables exist. - Schema drift is handled defensively with
field_exists(),table_exists(),method_exists(),class_exists()andstring_exists()guards, so optional/newer features degrade gracefully.
Low risk. There is no security impact: every statement is parameterised and scoped by blockinstanceid and/or userid, and the current user must already hold block/playerhud:view on the block before any of this runs. The only realistic failure mode is a future block_playerhud schema change breaking the filter — and even that is partially absorbed by the runtime existence checks. Blast radius is limited to the filter failing to render (fail-closed), never to data exposure or corruption.
The filter is explicitly a companion to block_playerhud and cannot function without it (declared in version.php as $plugin->dependencies = ['block_playerhud' => 2026090301]). The direct reads exist to bulk-load drop, inventory and media data in one pass (the plugin's documented "Zero N+1" strategy in preload_data()), and the author has clearly anticipated schema variance across block versions — hence the pervasive *_exists() guards and the value-column fallback ('1 as value').
$sql = "SELECT d.id as dropid, d.maxusage, $valuecolumn, d.respawntime, d.blockinstanceid, d.code,
i.id as itemid, i.name as itemname, i.image, i.xp, i.description,
i.secret, i.required_class_id
FROM {block_playerhud_drops} d
JOIN {block_playerhud_items} i ON d.itemid = i.id
WHERE d.code $insql
AND d.blockinstanceid = :bi
AND i.enabled = 1";
Prefer the block's public API where one exists. The plugin already calls \block_playerhud\game::, \block_playerhud\utils:: and block_playerhud_get_drop_details_by_code() in places; extending that surface (e.g. a bulk drop/inventory loader in the block) would let the filter drop the raw table reads while keeping the Zero-N+1 goal. Where a raw read is genuinely required for performance, keep the existing field_exists/table_exists guards and document the schema contract the filter relies on so both plugins are versioned together.
$sqlinv = "SELECT itemid, COUNT(id) as qty FROM {block_playerhud_inventory} WHERE userid = :userid GROUP BY itemid";
$player = $DB->get_record('block_playerhud_user', [
'blockinstanceid' => $this->instance->id,
'userid' => $USER->id,
]);
render_drop() concatenates the drop card's data-* attributes into a single raw HTML string ($dataattributes) which is then output through unescaped triple-mustache ({{{data_attributes}}}) in drop.mustache.
This is safe as written today — every interpolated value is either wrapped in s() (data-name, data-image, data-xp, data-progress-text, data-qty-text, data-respawntime-str), a base64 string (data-desc-b64), or a value the code treats as an integer (data-isimage, data-unique, data-timestamp, data-maxusage).
The observation is about fragility / defence in depth, not a live bug:
- Building attribute markup by hand and injecting it with
{{{ }}}bypasses the template engine's auto-escaping safety net. A future edit that appends a new value withouts()would introduce stored XSS with no template-level backstop. - Two attributes (
data-maxusage,data-timestamp) already rely on the column being integer-typed rather than on explicit escaping — correct now, but an implicit contract.
Informational. No exploit exists in the current code — all dynamic values reaching {{{data_attributes}}} are escaped or integer-typed, and descriptions are separately cleaned with format_text() before base64 encoding. The note is a defence-in-depth recommendation: moving escaping into the template (or documenting the invariant) removes the risk that a later change silently introduces an unescaped attribute.
The data-* attributes carry the item's name, description (base64), image, XP and progress figures to block_playerhud's client-side filter_collect.js, which reads them to populate the item-details modal. The values originate from teacher-authored item records and from the drop's numeric configuration. The plugin's own PHPUnit test test_drop_template_escapes_media_content() and Behat step the_playerhud_filter_modal_description_html_should_not_contain show the author actively guards the escaping of this render path.
$dataattributes = 'data-name="' . $safename . '" ' .
'data-desc-b64="' . $htmldesc . '" ' .
'data-image="' . s($rawimage) . '" ' .
'data-isimage="' . ($media['is_image'] ? 1 : 0) . '" ' .
'data-xp="' . s($xpdisplay) . '" ' .
'data-unique="' . ($isunique ? 1 : 0) . '" ' .
'data-timestamp="' . $timestamp . '" ' .
'data-progress-text="' . s($progresstext) . '" ' .
'data-qty-text="' . s($qtytext) . '" ' .
'data-maxusage="' . $data->maxusage . '" ' .
'data-respawntime-str="' . s($respawntimestr) . '"';
Pass discrete values to the template and let Mustache escape them. Replace the single {{{data_attributes}}} sink with named keys (data-name="{{data_name}}" data-xp="{{data_xp}}" ...) so every attribute is auto-escaped at render time and the escaping cannot be forgotten in a future edit. If the single-string approach is kept for template simplicity, route the currently-unescaped integers through s((string)$data->maxusage) / s((string)$timestamp) and add a short comment stating the invariant that every value in this string must be pre-escaped.
No third-party libraries are bundled in the shipped plugin, so thirdpartylibs.xml is correctly absent. The only non-Moodle JavaScript in the repository (docs/assets/js/carousel.js, lightbox.js, scrollspy.js) belongs to the plugin's Jekyll/GitHub-Pages documentation site and is excluded from the release archive by .gitattributes (docs/ export-ignore, .github/ export-ignore) — it is neither distributed nor loaded by Moodle, so it is out of scope for the runtime review.
The auto-enable-on-install behaviour is the sanctioned core pattern. db/install.php calls filter_set_global_state('playerhud', TEXTFILTER_ON), mirroring core filters filter_urltolink, filter_activitynames, filter_mediaplugin and filter_mathjaxloader, all of which enable themselves the same way in their own db/install.php. This is expected for a shortcode filter and is not a concern.
Privacy API usage is appropriate. The filter stores no personal data of its own — it only reads the block's tables and renders — so \core_privacy\local\metadata\null_provider with a privacy:metadata reason string is the correct implementation, and the string exists in both language packs.
Strong, security-aware test coverage. The tests/ suite includes explicit regression tests for the description-XSS sanitisation, the block/playerhud:view capability gate, shortcode re-entrancy/recursion, configdata object injection (via a __wakeup probe fixture), null descriptions, and template auto-escaping — in addition to functional and performance (N+1) tests. This materially raises confidence in the hardening.
Filter context handling. The filter resolves the target block via the $COURSE global rather than the passed $this->context. This is a common and acceptable pattern for course-scoped filters and fails closed (shortcodes are stripped) when no matching block is found in the current course, so it carries no security consequence; it is noted only for awareness.