Social Comments
block_socialcomments
Block plugin that adds a social commenting area to course and activity pages. Users can post comments (optionally scoped to a group), reply, pin comments or whole pages to a personal pinboard, and subscribe to pages to get scheduled digest notifications. It also provides a course-level comments report and newsfeed, optional real-time refresh through block_deft, backup/restore, and a full Privacy API implementation.
No security vulnerabilities were found after tracing every entry point.
- Web services (
classes/external.php) all validate parameters, resolve the context from the stored record rather than from client input when editing or replying, callvalidate_context()(which runsrequire_login()with the course and cm, as confirmed in core), and check capabilities. Ownership checks cover edits, and group visibility (can_view()) covers replies and pins. - Pages (
report/,pinboard/) callrequire_login()andrequire_capability(). The delete page enforcesrequire_sesskey()and checks that the comment belongs to the course. - Output: comment content always goes through
format_text(). Names go throughformat_string(), and the templates rely on mustache escaping. All SQL is parameterised, and section and activity names are bound as parameters even inside the report'sUNION ALL. - Digest and reports re-check enrolment, activity visibility and separate-groups membership for every comment.
What remains are a few low-severity issues: a broken, undocumented CSV download path on the report page, the hidepins block setting being enforced only in the UI, and double-escaped group and activity names. There is also one info-level note about reading mod_lti's table directly. The privacy provider is complete, and the plugin has good PHPUnit and Behat coverage, including access-control tests.
block_socialcomments is a mid-sized block (about 9,000 lines including tests) that adds threaded comments, pins, subscriptions with scheduled digests, a course comments report and optional real-time refresh to course and activity pages. It exposes nine AJAX web services, three report/pinboard pages and a scheduled task.
Overall it is in good security shape. Every web service validates the context and checks capabilities. When a comment or reply is edited, its context comes from the database record, not from the client. Separate-groups visibility is enforced when posting, replying, pinning and in the reports and digest. All SQL is parameterised, and user content is always passed through format_text() before output. I found no XSS, SQL injection, CSRF or authorisation bypass.
The findings are all low or info severity. The most notable one: the report page responds to an undocumented ?download= parameter, which mixes the HTML page into a file download. It does not leak data, because results are still filtered by visibility, but it is broken code. The hidepins block setting only hides the UI and is not enforced server-side. A few already-formatted names are escaped twice.
The real-time feature delegates token handling to block_deft, which is not part of this review. The plugin's own side, the renew_token service, is capability-checked and sends only a context id.
Findings
report/index.php passes any download request parameter to flexible_table::is_downloading(). In core this immediately calls is_downloadable(true) and starts the dataformat document, which sends download headers. The page then calls is_downloadable(false) and still prints $OUTPUT->header(), the tab tree and the filter form. In download mode it never prints the footer, and it never exits after finish_output(). The result is a download stream that mixes HTML page chrome with CSV/Excel rows, or an exception for an unknown dataformat. No download button is ever shown, so the code path is effectively dead but still reachable.
Low risk. The defect is functional, not a security issue: data is still filtered by report_helper visibility checks, and only users with block/socialcomments:viewreport can reach the page.
The report lists all comments the current user may see across the course's visible activities. The data passed to the export is the same visibility-filtered set, so the defect is broken output rather than disclosure.
$download = optional_param('download', '', PARAM_ALPHA);
if ($download) {
$table->is_downloading($download, userdate(time(), '%Y-%m-%d-%H%M%S') . '_report');
}
Either remove the download branch entirely (and the $download handling at lines 144, 164, 172 and 192), or support it properly: call is_downloading() first, skip $OUTPUT->header(), the tabs and the form when downloading, and leave is_downloadable() set to true so the download buttons are rendered.
$download = optional_param('download', '', PARAM_ALPHA);
$table->is_downloading($download, 'socialcomments_report');
...
if (!$table->is_downloading()) {
echo $OUTPUT->header();
...
}
$table->is_downloadable(false);
The block instance setting Hide pins (config_hidepins) is read in render_block_content() and passed to the JavaScript. The JavaScript sends it back as the hidepins parameter of block_socialcomments_get_commentspage, and the server uses that value as-is (the unvalidated $hidepins argument rather than $params['hidepins']). set_pinned checks only the pinitems capability and ignores the block configuration. A user can therefore send hidepins=0 to get the pin controls back, or call set_pinned directly, even when the teacher turned pins off for that block.
Low risk. The only result is that a user pins items for themselves despite a teacher's UI preference. No other user's data is exposed or changed.
Pins are private, per-user bookmarks shown only on that user's pinboard. The capability check and comment visibility check still apply, so the only effect is bypassing a display preference.
$config['hidepins'] = $hidepins;
$commentshelper = new \block_socialcomments\local\comments_helper($context, $config);
Use the validated parameter at minimum ($params['hidepins']). Ideally, look up the block instance configuration for the page context on the server, rather than trusting a client flag, and apply it in both get_commentspage and set_pinned.
require_capability('block/socialcomments:pinitems', $context);
format_string() returns HTML-escaped text (core replaces & and escapes < and >). In several places that output is escaped a second time:
- Group names in the comment form are
format_string()ed in the renderer, then output with{{name}}inform.mustache. - In
render_activity(), the fallback wraps the already-formatted activity name ins().
Names containing &, < or > (e.g. Q&A) are displayed literally as Q&A.
Low risk. This is a cosmetic display bug with no security impact. Double escaping is safe but shows entity text to users.
These are display paths for group and activity names in the block form and the comments report. Both values are already sanitised by core before they reach the plugin.
'name' => format_string($group->name, true, ['context' => $context]),
Output the formatted value unescaped in the template ({{{name}}}) or pass ['escape' => false] to format_string() and keep {{name}}.
{{#groups}}<option value="{{id}}">{{name}}</option>{{/groups}}
return s($comment->activity);
$comment->activity comes from cm_info::get_formatted_name() and is already safe HTML; return it without s().
realtime::enabled() decides whether real-time updates are active by querying lti_types for a row with tooldomain = 'deftly.us'. That table belongs to mod_lti and the registration is managed by block_deft. The plugin now depends on internal storage details of two other components. The code comment says this mirrors what block_deft does itself. The dml_exception catch handles a missing table, but a schema or registration change in either component would silently disable real-time updates.
Informational. The query is a read-only literal with no user input, so there is no security impact; the concern is maintainability only.
This check gates whether the block issues real-time tokens and dispatches change notifications. It is cached per request in a static variable.
$registered = (bool) $DB->get_field_select('lti_types', 'clientid', "tooldomain = 'deftly.us'");
Prefer an API exposed by block_deft (or ask its maintainers to add one, e.g. \block_deft\socket::is_registered()) instead of reading mod_lti storage directly.
Real-time updates depend on block_deft, which is not part of this review. classes/socket.php extends \block_deft\socket, and amd/src/realtime.js imports block_deft/socket. Token generation and dispatch to the external Deft service happen in that plugin. This plugin's side is sound: renew_token requires block/socialcomments:view on a validated course or module context. According to the privacy metadata and code comments, only the context id is sent to the service, and pages then reload comments through the permission-checked get_commentspage service.
save_comment fails silently on a permission check. When can_save() returns false, it returns id => 0 instead of throwing like the other services. The JavaScript then clears the textarea and reloads, so the user's text is lost without any message. This can only happen through crafted requests or a race (e.g. group membership removed while the page is open), but throwing nopermissions would be more consistent.
Stale comments: the docblock of block_socialcomments_extend_navigation_course() in lib.php mentions moodle/site:viewreports, but the code checks block/socialcomments:viewreport. The upgrade step 2026030701 says the digest task "runs every minute now", but db/tasks.php schedules it every 15 minutes.
Test coverage is strong. The PHPUnit tests explicitly cover cross-course reply attempts, separate-groups pin/reply denial, SQL-quote handling in report names, hidden-activity filtering in the newsfeed, and digest access after unenrolment or hiding an activity, plus backup/restore and the full privacy provider.