🕹️PlayerHUD
block_playerhud
PlayerHUD is a gamification block for Moodle that adds a heads-up display, XP/levels, an item collection system with drop locations and shortcodes, quests tied to course activities or game milestones, a shop/trade economy, RPG classes with karma and branching story chapters, leaderboards, and AI-assisted content generation (Gemini, Groq, or any OpenAI-compatible endpoint). It exposes a teacher management panel, a student view with multiple tabs, web services for AJAX interactions, a CSV/Excel grade export, backup/restore support, and a Privacy API provider for GDPR compliance.
The plugin is well engineered and demonstrates a strong grasp of Moodle security idioms.
Strengths observed across the codebase:
- Every entry-point script calls
require_login(), thencontext_block::instance()andrequire_capability()before touching data. - State-changing actions are gated by
confirm_sesskey()or usemoodleform/dynamic_formwhich handle sesskey automatically. - All DB access goes through the
$DBAPI with named/positional placeholders — no string concatenation of user input into SQL. - Output uses
format_string()/format_text()consistently, mustache templates rely on auto-escaping, and JavaScript that injects DOM usestext()/textContentfor untrusted values. - The AI generator uses Moodle's
curlwrapper (so the core SSRF security helper is engaged) and adds an additional layer of HTTPS-only validation plus DNS-resolved private/loopback address rejection for teacher-configured endpoints. - The Privacy API provider implements
plugin\provider,core_userlist_provider, anduser_preference_providerwith all stored tables and external AI endpoints declared. - A
lock_factorymutex serialises trade execution per user, and shop/quest/trade operations usestart_delegated_transaction()with rollback on failure. - Capability definitions correctly carry
RISK_XSS/RISK_SPAMbitmasks, the management capability is teacher-gated, and view actions verify enrollment before allowing teacher-initiated changes on other users.
Findings driving the grade: all are low-severity code-quality issues — non-portable LIMIT 1 in raw SQL (would fail on MSSQL/Oracle), missing per-instance validation of foreign-key item IDs submitted through teacher-only quest/trade forms (limited blast radius since teachers already control the instance), and a small XP-accounting bug when revoking items originating from infinite drops. None are exploitable by students or unauthenticated users.
block_playerhud 5.2 is a large gamification block (~30 PHP entry points, 16 service classes, 16 AMD modules, 14 DB tables, 9 external web service functions) that displays an RPG-style sidebar with XP, levels, items, quests, classes, karma, branching stories, trades, and leaderboards.
Security posture: strong.
- The plugin consistently applies Moodle's defence-in-depth pattern:
require_login→context_block::instance→require_capability→confirm_sesskeyon state changes. - Web service methods all call
validate_parameters,validate_context, and the appropriate capability check; the AI generation methods additionally verify the user hasblock/playerhud:manage. - AI content generation goes through Moodle's
curlclass (which engages the corecurl_security_helper) and adds an extra HTTPS-only check plus DNS-resolved private/loopback rejection on teacher-supplied base URLs. - File handling uses the Moodle File API exclusively (
get_file_storage,send_stored_file,make_pluginfile_url,file_save_draft_area_files). No direct writes to$CFG->dataroot/filediror$CFG->dirroot. - The backup/restore provider covers all 14 tables, uses namespaced ID mappings to avoid collisions with core, and defers cross-references for after_execute fixup.
- The Privacy API provider implements the full set of interfaces (
plugin\provider,core_userlist_provider,user_preference_provider), declares each table with its translated fields, and lists the three external AI endpoints asadd_external_location_link.
Findings: all three reported issues are low-severity and non-exploitable by students or unauthenticated users:
- Raw SQL uses
LIMIT 1in two chapter-reorder queries — works on MySQL/PostgreSQL but breaks on MSSQL/Oracle. TheLIMIT 1is also redundant becauseget_record_sqlalready enforces it. - The quest and trade edit forms accept
req_itemid,reward_itemid,req_itemid_$i, andgive_itemid_$iwithout verifying the submitted IDs belong to the current block instance. Only teachers withblock/playerhud:managecan reach the form, so the practical blast radius is small. - The
revoke_itemadmin action subtracts$item->xpfrom the player's current XP unconditionally, but the inventory entry may have originated from an infinite drop (whereprocess_collectiondeliberately granted 0 XP). The revoke can therefore push a player's XP lower than it should be (themax(0, …)clamp limits the worst case).
No critical, high, or medium findings were identified. No SQL injection, XSS, authentication bypass, capability bypass, file write outside the File API, code execution sink, or hardcoded credentials were found.
Findings
Two get_record_sql() calls in the chapter reorder actions embed LIMIT 1 directly in the SQL string. LIMIT syntax is supported by MySQL/MariaDB, PostgreSQL, and SQLite, but it is not valid on Microsoft SQL Server (which uses TOP / OFFSET … FETCH NEXT) or Oracle (which uses ROWNUM or FETCH FIRST). Moodle officially supports all of these engines, so the plugin will throw a fatal SQL error when reordering chapters on an MSSQL or Oracle Moodle install.
The LIMIT 1 is also redundant: get_record_sql() already passes $limitnum = 1 internally, so the row is already capped to one regardless of the SQL clause. Removing the literal LIMIT 1 makes the queries portable without any behavioural change.
Low risk (portability). No security implication. The user-visible failure is that teachers on MSSQL- or Oracle-backed Moodle sites will see a fatal SQL error when trying to reorder chapters, and the plugin will not be deployable on those engines without patching.
These queries are triggered by the Move chapter up / Move chapter down actions in the management panel (tab=chapters). The handler in manage.php reads the current chapter, finds the immediate neighbour by sortorder, and swaps the two sortorder values to reorder the chapter list.
On MySQL/MariaDB/PostgreSQL/SQLite the SQL parses fine, so functional testing on these engines passes — which is likely why the bug has not surfaced.
$prevchapter = $DB->get_record_sql(
"SELECT * FROM {block_playerhud_chapters}
WHERE blockinstanceid = ? AND sortorder < ?
ORDER BY sortorder DESC
LIMIT 1",
[$instanceid, $chapter->sortorder]
);
Drop the literal LIMIT 1 — get_record_sql() already enforces a single row.
$prevchapter = $DB->get_record_sql(
"SELECT * FROM {block_playerhud_chapters}
WHERE blockinstanceid = ? AND sortorder < ?
ORDER BY sortorder DESC",
[$instanceid, $chapter->sortorder]
);
$nextchapter = $DB->get_record_sql(
"SELECT * FROM {block_playerhud_chapters}
WHERE blockinstanceid = ? AND sortorder > ?
ORDER BY sortorder ASC
LIMIT 1",
[$instanceid, $chapter->sortorder]
);
Same fix — remove the redundant, non-portable LIMIT 1:
$nextchapter = $DB->get_record_sql(
"SELECT * FROM {block_playerhud_chapters}
WHERE blockinstanceid = ? AND sortorder > ?
ORDER BY sortorder ASC",
[$instanceid, $chapter->sortorder]
);
Several teacher-facing forms accept foreign-key IDs (req_itemid, reward_itemid, req_tradeid, req_itemid_$i, give_itemid_$i) and write them into the database without confirming the referenced row belongs to the current block instance.
The HTML <select> elements are populated with the correct instance-scoped list, but a teacher who tampers with the POST request — for example via the browser devtools or curl — can substitute an item ID owned by a different block_playerhud instance (in another course they also teach, or even one they cannot access if they can guess an ID). The resulting trade or quest will reference that foreign row.
The trade execution path in trade_manager::execute_trade joins on block_playerhud_items without an instance filter, so the foreign reference is dereferenced at runtime, producing requirements / rewards that show another instance's item name and consume / grant inventory rows of the cross-instance item.
The practical blast radius is small because reaching these forms requires the block/playerhud:manage capability, but the integrity check would normally be expected for forms that accept numeric references.
Low risk. Exploitation requires the block/playerhud:manage capability — a role that is already trusted to add, edit, and delete game content in this instance. The worst-case impact is logical / cosmetic: trades or quests reference items the local players will see but not necessarily understand, and consuming those items may operate against another instance's inventory rows. There is no privilege escalation, no data leak beyond an item name, and the action is auditable via Moodle's standard logs. Adding the cross-instance check is straightforward and brings these forms in line with the rest of the plugin.
Both forms are reachable only from /blocks/playerhud/manage.php and /blocks/playerhud/edit_trade.php, which start with require_capability('block/playerhud:manage', $context) on the current block instance. The teacher is therefore already trusted to manage this instance's content, but the dropdown options in the rendered form are restricted to the current instance — overriding them via crafted POST data is the only way to reach a cross-instance ID.
The rest of the codebase (e.g. delete actions, drops controller, item edit) does enforce blockinstanceid on every update — see for example classes/controller/drops.php:394-403 which explicitly preserves the ownership fields from the DB record on update.
- As a teacher with manage capability on block instance A (course X) and another teacher account or a previously seen ID from block instance B (course Y), open the Trades tab in course X and start editing or creating a trade.
- Use browser devtools to change the
valueof one of the hidden<select>req_itemid_0options to a numeric ID belonging to course Y'sblock_playerhud_items, then submit the form. - The trade is saved with
itemidpointing to course Y's item. Opening the shop in course X now displays that foreign item's name as a trade requirement.
$reqstoinsert = [];
for ($i = 0; $i < $data->repeats_req; $i++) {
$itemfield = "req_itemid_$i";
$qtyfield = "req_qty_$i";
if (!empty($data->$itemfield) && $data->$itemfield > 0) {
$req = new \stdClass();
$req->tradeid = $currenttradeid;
$req->itemid = $data->$itemfield;
$req->qty = max(1, (int) $data->$qtyfield);
$reqstoinsert[] = $req;
}
}
Resolve the valid item-ID set for this instance once, then reject submissions that reference IDs outside it:
$validitemids = $DB->get_fieldset_select(
'block_playerhud_items',
'id',
'blockinstanceid = ?',
[$instanceid]
);
foreach (range(0, $data->repeats_req - 1) as $i) {
$itemid = (int)($data->{"req_itemid_$i"} ?? 0);
if ($itemid <= 0 || !in_array($itemid, $validitemids, true)) {
continue;
}
// …build $req as before…
}
Apply the same in_array($id, $validitemids, true) check to the give_itemid_$i loop below it.
$record->reward_itemid = (int)$data->reward_itemid;
$record->required_class_id = '0';
$record->image_todo = trim($data->image_todo ?? '');
…
if ($type === quest::TYPE_SPECIFIC_ITEM) {
$record->req_itemid = (int)$data->req_itemid;
} else if ($type === quest::TYPE_SPECIFIC_TRADE) {
$record->req_itemid = (int)$data->req_tradeid;
}
Validate that the supplied reward_itemid, req_itemid, and req_tradeid (for TYPE_SPECIFIC_TRADE) belong to $this->instanceid before writing the record:
if ((int)$data->reward_itemid > 0) {
$exists = $DB->record_exists('block_playerhud_items', [
'id' => $data->reward_itemid,
'blockinstanceid' => $this->instanceid,
]);
$record->reward_itemid = $exists ? (int)$data->reward_itemid : 0;
}
Repeat for req_itemid against block_playerhud_items and req_tradeid against block_playerhud_trades.
When a teacher revokes a student's collected item via the management report, the handler subtracts the item's full xp value from the student's currentxp:
process_collection()inclasses/game.phpdeliberately grants 0 XP when$drop->maxusage === 0(an infinite drop) — comment on line 155: "Infinite drops (0 maxusage) give 0 XP to prevent farming."revoke_iteminmanage.phpdoes not load the originating drop nor check whether it was infinite. It always subtracts$item->xp.
As a result, revoking an inventory entry that came from an infinite drop removes XP that was never granted in the first place. The max(0, …) clamp prevents negative totals but the student still loses XP they earned legitimately from other drops/quests.
This is not exploitable by students — only teachers/managers reach this action — but the inconsistency makes the audit trail and gradebook export wrong.
Low risk. This is a data-integrity bug rather than a security vulnerability. A teacher who revokes items in good faith on a long-running course can quietly lose XP from students; affected students may see their level drop without an explanation. The max(0, …) clamp prevents negative balances, but tie-breaker timestamps (timemodified) are not adjusted, which can also distort the ranking.
The flow is: teacher opens the Reports → user audit view, sees a row of the student's inventory marked source='map', clicks the revoke (trash) icon, and the handler runs. The reverse symmetric of process_collection — which is the function that originally populated the row — is what should be applied, but only the item lookup is symmetric, not the XP rule.
$inv = $DB->get_record_sql(
"SELECT inv.*
FROM {block_playerhud_inventory} inv
JOIN {block_playerhud_items} i ON i.id = inv.itemid
WHERE inv.id = :invid AND i.blockinstanceid = :instanceid",
['invid' => $invid, 'instanceid' => $instanceid]
);
if ($inv) {
$item = $DB->get_record('block_playerhud_items', ['id' => $inv->itemid, 'blockinstanceid' => $instanceid]);
if ($item) {
$player = $DB->get_record('block_playerhud_user', ['blockinstanceid' => $instanceid, 'userid' => $inv->userid]);
if ($player) {
$player->currentxp = max(0, $player->currentxp - $item->xp);
$DB->update_record('block_playerhud_user', $player);
}
…
}
}
Look up the source drop and only deduct XP when it was finite, mirroring the rule applied in process_collection:
$isinfinite = false;
if ($inv->dropid > 0) {
$drop = $DB->get_record('block_playerhud_drops', ['id' => $inv->dropid]);
$isinfinite = $drop && (int)$drop->maxusage === 0;
}
if (!$isinfinite) {
$player->currentxp = max(0, $player->currentxp - $item->xp);
$DB->update_record('block_playerhud_user', $player);
}
Quest/teacher-granted inventory (source = 'quest' or 'teacher') is another edge case — for those the original XP was awarded directly via quest claim, not via the drop path. Consider whether the revoke action should mirror that grant too (e.g. quest_log claims also adjust currentxp).
The admin setting (settings.php) and the teacher save action (manage.php → action=save_keys) both validate openai_url with PARAM_URL, which accepts any well-formed URL including http:// schemes. The runtime check in classes/ai/generator.php::is_safe_url then rejects anything that is not HTTPS by silently blanking the URL out (so the provider falls back to the next configured one).
The behaviour is safe — an http:// URL never reaches the network — but the user experience is poor: an admin or teacher can save an http:// endpoint, no error is shown, and AI generation silently fails over to the next provider with no diagnostic.
This is informational only.
Informational. No security impact; the URL is dropped before reaching the network. Only worth fixing for usability.
is_safe_url performs strong SSRF prevention: it requires https, blocks localhost/127.0.0.1/::1, blocks RFC-1918 / link-local / reserved-range IPs, and re-resolves the hostname via dns_get_record to defeat DNS-rebinding tricks. Moodle's \curl class additionally engages the core security helper at request time, so the protection is layered.
$settings->add(new admin_setting_configtext(
'block_playerhud/openai_baseurl',
get_string('openai_baseurl', 'block_playerhud'),
get_string('openai_baseurl_desc', 'block_playerhud'),
'',
PARAM_URL
));
Either tighten validation at save-time (custom validate() in an admin_setting_configtext subclass that requires https://), or surface a translated error when is_safe_url() rejects a URL at runtime so the operator knows why AI calls are silently bypassing the custom endpoint.
get_audit_logs() in classes/output/manage/tab_reports.php (and its duplicate in classes/output/view/tab_history.php) constructs short HTML fragments in PHP and passes them to mustache as xp_badge and details_html. The templates render those fields with triple-brace unescaped syntax (e.g. {{{xp_badge}}}, {{{details_html}}}).
Reviewing the constructed strings shows every interpolation is either a server-controlled integer ($log->xp_gained), a static get_string() result, or a value processed by format_string(). There is therefore no XSS today.
This is informational because future maintainers who add another field into the HTML concatenation can easily forget to escape it and introduce an XSS sink. The standard Moodle pattern is to move the HTML into the mustache template so all interpolations are auto-escaped, and only pass plain data fields.
Informational / maintainability. No exploitable XSS exists today. Refactoring the HTML out of PHP into mustache eliminates the risk that a future change adds an unescaped field.
The data fed into the HTML is currently safe: xp_gained is an integer computed in SQL via CASE WHEN, details is either a known-internal source string ('map', 'revoked', 'consumed', 'trade_completed', 'quest_claim') or a get_string result, and item names are run through format_string before being concatenated.
if (isset($tradecosts[$log->trade_id])) {
$coststr = implode(', ', $tradecosts[$log->trade_id]);
$strcost = get_string('trade_cost', 'block_playerhud');
$iconminus = '<i class="fa fa-minus-circle" aria-hidden="true"></i>';
$detailtext .= "<small class=\"text-danger d-block mt-1 text-wrap\">" .
"{$iconminus} {$strcost} {$coststr}</small>";
}
Move the HTML scaffold into the mustache template and pass only the data:
$rowdata['has_trade_cost'] = isset($tradecosts[$log->trade_id]);
$rowdata['trade_cost_str'] = $rowdata['has_trade_cost']
? implode(', ', $tradecosts[$log->trade_id]) : '';
In tab_history.mustache:
{{#has_trade_cost}}
<small class="text-danger d-block mt-1 text-wrap">
<i class="fa fa-minus-circle" aria-hidden="true"></i>
{{#str}} trade_cost, block_playerhud {{/str}} {{trade_cost_str}}
</small>
{{/has_trade_cost}}
Then details_html can become details rendered with {{details}} (double-brace auto-escape).
Defence-in-depth on AJAX endpoints. Every external service method (classes/external.php) calls validate_parameters, validate_context, and require_capability before touching state. insert_drop_shortcode and remove_drop_shortcode additionally enforce moodle/course:manageactivities on the target course context and reject when the block's parent course context differs from the supplied courseid. This is exactly the layered pattern recommended for Moodle web services.
SSRF protection is well thought through. classes/ai/generator.php::is_safe_url requires HTTPS, blocks localhost / 127.0.0.1 / ::1, runs the validated host through filter_var(... FILTER_FLAG_NO_PRIV_RANGE | FILTER_FLAG_NO_RES_RANGE), and for hostnames re-resolves all A/AAAA records and re-applies the private-range check to defeat DNS-rebinding. Because Moodle's \curl class also engages the core curl_security_helper at request time (with CURLOPT_RESOLVE pinning the validated IPs), there are two independent layers of SSRF defence.
Trade execution uses a per-user lock. classes/trade_manager.php::execute_trade acquires a \core\lock\lock_config::get_lock_factory('block_playerhud') mutex keyed by 'trade_usr_<userid>_inst_<instanceid>' before validating inventory and committing the transaction, with a try / finally that releases the lock. This prevents race-condition double-spends from concurrent shop submissions — a thoughtful touch that many plugins miss.
Backup/restore namespaces its ID mappings. The restore step uses keys like playerhud_item, playerhud_drop, playerhud_quest, playerhud_chapter, playerhud_node, playerhud_class for set_mapping / get_mappingid, avoiding collisions with core or other plugins' restore IDs. process_choice defers next_nodeid fixup to after_execute to cope with siblings that have not yet been restored — correct handling of the depth-first XML traversal.
Privacy provider is complete. The provider declares all eight tables that hold user data plus the five user preferences storing AI keys, lists three external locations (Gemini, Groq, OpenAI-compatible), and implements get_users_in_context, delete_data_for_users, delete_data_for_all_users_in_context, and delete_user_preferences. Bulk deletes use joins (block_playerhud_inventory → block_playerhud_items) so that orphan rows are not left behind even when only some user IDs are removed.
Capability risk bitmasks are correct. block/playerhud:addinstance carries RISK_XSS (teachers can add a block which can render rich content), block/playerhud:manage carries RISK_SPAM | RISK_XSS (manage panel writes item descriptions through FORMAT_HTML). View-only is captype => 'read' with no risk flag. These match Moodle's expectations for the role-permission audit pages.
Style note on unserialize_object(base64_decode($bi->configdata)). This is the canonical Moodle pattern for reading a block instance's persisted config, used by core itself in block_base::instance_config_load. Switching to \core\encoding\... is unnecessary and would actually break compatibility — the existing usage is correct.