Security: cross-course overview authorization bypass (BCP-SEC-001)

[cmassoglia]

Developer Finding Report: BCP-SEC-001 — Cross-course overview query caused by an unbound block instance and course

This report is an evidence-based remediation aid. Any code snippet is an informative suggestion and must be reviewed and tested in the target Moodle environment before use.

Field	Value
Plugin	block_completion_progress
Severity	High
Assessment impact	Security and privacy
Confidence	Likely
OWASP	A01:2025 - Broken Access Control
Moodle practice	Verify course and module access

Description

The controller accepts courseid and instanceid independently, loads both records, and runs require_login() for the requested course but require_capability('block/completion_progress:overview') for the supplied block context. It never verifies that block_instances.parentcontextid belongs to the courseid context. It then combines completion_progress for the requested course with the supplied block instance and builds the table for that courseid.

Affected Code

• overview.php — overview controller (lines 39-62)

• classes/table/overview.php — query_db (lines 162-297)

Evidence

• overview.php:39-62 — The identifiers and contexts are created independently; authorization does not bind the block instance to the requested course context.

$instanceid = required_param('instanceid', PARAM_INT);
$courseid = required_param('courseid', PARAM_INT);
$course = $DB->get_record('course', ['id' => $courseid], '*', MUST_EXIST);
$blockinstance = $DB->get_record('block_instances', ['id' => $instanceid], '*', MUST_EXIST);
$context = context_course::instance($courseid);
$blockcontext = context_block::instance($instanceid);
require_login($course, false);
require_capability('block/completion_progress:overview', $blockcontext);

• overview.php:71 — The requested course is combined with the uncorrelated block instance.

$progress = (new completion_progress($course))->for_overview()->for_block_instance($blockinstance);

• classes/table/overview.php:162-171,287-297 — The table uses the supplied course context to get enrolled users and identity fields, and the supplied block instance for cached progress.

$params = ['courseid' => $this->courseid];
$enroljoin = get_enrolled_with_capabilities_join($this->context, '', '', 0, !$showinactive);
$params['bi'] = $this->blockinstanceid;
SELECT DISTINCT {$userfields->selects}, l.timeaccess, b.percentage AS progress
... LEFT JOIN {block_completion_progress} b ON b.userid = u.id AND b.blockinstanceid = :bi

Steps to Reproduce

1. Create courses A and B, each with a Completion Progress block instance.
2. Give a test user block/completion_progress:overview on the block in B and enrol that user in A without overview permission there.
3. While authenticated as the test user, request /blocks/completion_progress/overview.php?instanceid=&courseid=.
4. Observe whether the participants and completion information for A are rendered or downloadable.

Expected Result

The cross-course pair must be rejected before data is queried; only an instance whose parent context is the requested course may be used.

Impact

An actor authorized to view an overview on a block in course B and able to log into course A can retrieve data about course A participants without holding overview permission there: name and permitted identity fields, last access, percentage, and completion states. This breaks course isolation and discloses personal academic data.

Suggested Remediation

Before authorization, verify that the instance belongs to the requested course context, for example by comparing $blockinstance->parentcontextid with $context->id and confirming blockname is completion_progress. Reject a mismatch, then check the capability on the correlated block context. Add a two-course regression test: a user with overview only in B and enrolment in A must be rejected for the cross-course pair while a legitimate pair continues to work.

Proposed Remediation Snippet (Informative)

The guard binds the requested course to the block instance before the block-context capability is accepted. It is informative source-checked guidance, not a verified patch.

$context = context_course::instance($courseid);
$blockinstance = $DB->get_record('block_instances', ['id' => $instanceid], '*', MUST_EXIST);
if ($blockinstance->blockname !== 'completion_progress' || (int)$blockinstance->parentcontextid !== (int)$context->id) {
    throw new required_capability_exception($context, 'block/completion_progress:overview', 'nopermissions', '');
}
$blockcontext = context_block::instance($instanceid);
require_login($course, false);
require_capability('block/completion_progress:overview', $blockcontext);

Production Impact

Exploitation requires a Moodle session and a role/enrolment combination across two courses, but the output contains multiple participants' academic data and supports download. It is not suitable for production until the relationship is enforced and validated on the target Moodle deployment.