Moodle Accounts in LDAP
report_ldapaccounts
An admin-level site report (report_ldapaccounts) that lists Moodle user accounts and checks whether each account's email address exists in a configured LDAP directory. It offers a filterable report with CSV export, quick action links to suspend/delete/notify users, a scheduled task and CLI script to create (synchronise) Moodle accounts from LDAP, and a second CLI script to bulk suspend/delete/set the email-stop flag on accounts that are missing from LDAP.
The plugin demonstrates a consistently strong security posture. All database access is parameterised through $DB placeholders, user-facing report output is escaped with format_string() (HTML escaping on by default), the CSV-download parameter is constrained with PARAM_ALPHANUM to prevent path traversal, the suspend/delete action links carry sesskey and defer the actual state change to core admin/user.php (which re-checks capability and sesskey), and CSV cells are prefixed to prevent spreadsheet-formula injection. index.php enforces require_login() plus a system-context capability check, both CLI scripts are shielded from web execution by define('CLI_SCRIPT', true), and account creation via the sync task/CLI runs only in an administrator context.
No exploitable vulnerabilities were found. A regular authenticated user cannot reach the report, cannot trigger the LDAP sync, and cannot harm other users. The only user-influenceable input that reaches an LDAP filter is the account owner's own email address, and Moodle's validate_email() blocks every structurally significant LDAP metacharacter except the wildcard *, whose effect is not observable to the user who placed it.
The findings are all low severity: an LDAP-filter escaping gap that is effectively neutralised by core email validation; a Privacy API null_provider declaration that understates the personal data written to temp-directory CSV and log files; defensive validation gaps in the reusable user_query builder that are not reachable with attacker input today; and a CI workflow whose static-analysis gates are disabled by a conditional mistake. Test coverage is extensive and the code follows Moodle conventions throughout.
Overview
report_ldapaccounts is a cleanly written site report for reconciling Moodle accounts against an LDAP directory. It reads users from the user table, checks their emails against LDAP, renders a filterable table (with CSV export and admin action links), and can create or deactivate accounts based on LDAP membership via a scheduled task and two CLI tools.
Security posture
The plugin gets the security-critical basics right:
- Access control —
index.phpcallsrequire_login()and checksreport/ldapaccounts:viewat system context; the capability defaults to themanagerarchetype. The sync task runs as the cron admin user and the SSO-sync CLI explicitly sets the admin user before creating accounts. - SQL — every query uses
$DBwith bound placeholders. User-supplied column names are whitelisted against realusercolumns and custom profile fields, and custom fields are resolved through the core\core_user\fieldsAPI (which binds field shortnames as parameters). - Output — report cells pass through
format_string(), which HTML-escapes by default; date columns useuserdate_htmltime(). CSV output guards against spreadsheet-formula injection. - File handling — the CSV download parameter is
PARAM_ALPHANUM, so path traversal is not possible; generated files live under$CFG->tempdir(the sanctioned pattern for plugin-generated files). - CLI — both CLI scripts define
CLI_SCRIPTand are therefore refused by core when invoked over the web.
Findings
All findings are low severity:
- LDAP filter values are not escaped with
ldap_escape(). The only low-privilege input that reaches a filter is the user's own email, and core email validation blocks all dangerous metacharacters except*, so this is a hardening gap rather than a live injection. - Privacy
null_providerunderstates reality — CSV exports and (opt-in) debug logs write personal data, including IP addresses, to the temp directory. user_querytrusts the operator element and the constructor$filterargument without full validation; not reachable with attacker input today, but fragile for a reusable component.- CI workflow disables its static-analysis/validation steps via
if: ${{ cancelled() }}, so only PHPUnit runs.
No medium, high or critical issues were identified, and no path allows an unauthenticated or low-privilege user to damage other users' data.
Findings
The ldap::get_filter() helper builds LDAP search filters by concatenating field names and values directly into the filter string, with no call to ldap_escape(). Values that reach this method include Moodle user email addresses read from the user table (the report queries LDAP with the emails of the matched users).
LDAP filter metacharacters (*, the parentheses, the backslash, and the NUL byte) placed in a value alter the structure or semantics of the query — the classic LDAP injection pattern.
In practice the exposure is tightly constrained: the only value a low-privilege user can influence is their own email address, and Moodle's validate_email() (PHPMailer's filter_var(FILTER_VALIDATE_EMAIL) validator plus a <> check) rejects the parentheses, the backslash and NUL, allowing only the wildcard * through. The residual issue is therefore a hardening gap rather than a directly exploitable injection, but the method should still escape the values it interpolates.
Low risk. The method genuinely interpolates untrusted data into an LDAP filter without escaping, but the exploitable surface is minimal. The report is reachable only by users with report/ldapaccounts:view (manager), and the sole value a low-privilege user can influence — their own email — is filtered by Moodle's validate_email(), which permits only the * wildcard among LDAP metacharacters. A wildcard in the value cannot restructure the filter and produces no result that maps back to the attacker's row, so there is no information disclosure to the attacker and no impact on other users. The finding is a defence-in-depth/robustness fix: should emails ever enter the database through a channel that bypasses validate_email(), escaping here would prevent that from becoming a genuine injection.
The report page collects the email addresses of the Moodle users matching the manager's filter and passes them as the LDAP search values. get_filter() then emits (mail=<email>) clauses. $ldapmailfield and the fixed query part are administrator settings (trusted); the email values originate from the user table and can contain characters chosen by the account owner. The LDAP response is used only to set a boolean ldap_status per row that is displayed to the manager — it is never returned to the user who owns the email.
A self-registering user sets their profile email to victim*@example.com. When a manager later runs the report over a user set that includes this account, the plugin emits the filter clause (mail=victim*@example.com), in which * is an unescaped LDAP wildcard rather than a literal character. Structurally dangerous characters such as ( and ) cannot be used because validate_email() rejects them, so the payload cannot break out of the value to create new filter clauses, and the account owner never sees the report output.
foreach ($searchfields as $key => $value) {
if (is_array($value)) {
$filter .= '(|';
foreach ($value as $val) {
$filter .= '(' . $key . '=' . $val . ')';
}
$filter .= ')';
} else {
$filter .= '(' . $key . '=' . $value . ')';
}
}
Escape every interpolated value (and, defensively, the attribute name) for the LDAP filter context:
foreach ($searchfields as $key => $value) {
$key = ldap_escape($key, '', LDAP_ESCAPE_FILTER);
if (is_array($value)) {
$filter .= '(|';
foreach ($value as $val) {
$filter .= '(' . $key . '=' . ldap_escape($val, '', LDAP_ESCAPE_FILTER) . ')';
}
$filter .= ')';
} else {
$filter .= '(' . $key . '=' . ldap_escape($value, '', LDAP_ESCAPE_FILTER) . ')';
}
}
Note: continue to pass the administrator-configured fixedquerypart as a raw filter fragment — only the per-field search values need escaping.
$ldapres = $ldap->search([$ldapmailfield => array_keys($userinldap)], $ldapmailfield, $ldapquerypart)
->get_parsed_result();
No change needed at the call site once get_filter() escapes its inputs; the email values passed here are the untrusted data that must be escaped downstream.
The plugin's Privacy API provider implements \core_privacy\local\metadata\null_provider, which formally asserts that the plugin stores no personal data. In practice the plugin writes personal data to the server filesystem:
- CSV exports (
user_table::output_csv()) contain user IDs, usernames, email addresses, names and any other selected profile fields. - Debug logs (
ldap::log(), enabled by theloggingadmin setting) record the LDAP filter, the full LDAP result set (emails and directory attributes), the requesting user's ID and their IP address.
Both are written under $CFG->tempdir/report_ldapaccounts/. The plugin's own privacy:metadata language string acknowledges this ("personal data are written into the log file and the csv export file"), which is inconsistent with the null_provider contract.
Low risk. No data is exposed to unauthorised parties: both features require a privileged user, logging is opt-in, and the files reside in the protected temp directory. The issue is one of privacy accuracy/compliance rather than a data breach — the null_provider declaration understates the plugin's data handling, and the generated files are never removed by the plugin (they rely on Moodle's temp purge). Moodle plugin-directory privacy reviews commonly flag exactly this mismatch.
Logging is disabled by default and the CSV export is an explicit action by a user holding report/ldapaccounts:view (manager). Files live in Moodle's temp directory, which is periodically purged, and are only web-served through the capability-gated, PARAM_ALPHANUM download in index.php.
class provider implements \core_privacy\local\metadata\null_provider {
Reconcile the declaration with the plugin's behaviour. Options:
- Preferred: delete the CSV export after it is downloaded and avoid persisting personal data in the debug log (or hash/omit identifiers), so that
null_provideris genuinely accurate. - Otherwise, document the temp-directory files clearly and confirm with the site's data-protection process that a
null_provideris defensible given the opt-in, privileged, transient nature of the files.
if (!$fp = @fopen($this->get_logfile(true), 'a')) {
return;
}
When logging is enabled, the log accumulates emails, LDAP attributes, requesting user IDs and IP addresses in a dated file. Consider rotating/limiting retention and documenting it in the privacy metadata.
$this->csvfile = md5(microtime()) . '.csv';
$fp = fopen(config::get_plugin_file_dir() . DIRECTORY_SEPARATOR . $this->csvfile, 'w');
Remove the CSV file once it has been streamed to the requester in index.php, so that personal data is not left on disk until the temp purge.
user_query builds its SQL WHERE clause with two constructs that trust their inputs:
- Operator pass-through — in
get_where()the comparison operator ($val[1]) is concatenated straight into the SQL string. The operator is derived inset_filter_json()from the leading character(s) of a value and is currently limited to>,<,>"and<". The quoted variants inject a stray double quote into the SQL and yield a malformed statement (a runtimedml_exception) — not attacker-controlled SQL. - Unvalidated constructor filter —
set_filter()/set_filter_json()callvalidate_fields()to whitelist column names, but the constructor assigns$this->filter = $filterdirectly with no validation. Such a filter would flow intoget_where()as'u.' . $col, an unvalidated identifier.
No shipped caller feeds untrusted data into either path — the web report and both CLI scripts always go through set_filter_json(), whose keys are whitelisted and whose operators are bounded — so this is a robustness/defence-in-depth issue, not a live vulnerability.
Low risk. Not exploitable with any input an attacker can currently reach: values are always bound as parameters, column names are whitelisted on every used path, and the reachable operators only allow a self-inflicted SQL syntax error by a manager. The concern is that user_query is a reusable component whose public surface (the constructor $filter argument and the free-form operator) would permit unsafe SQL if a future caller passed untrusted data. Tightening both closes the gap.
On every used path the filter is built via set_filter_json(): column keys are validated against real user columns and custom profile fields, text values are parameterised through $DB->sql_like() or a placeholder, and the operator can only become one of a small fixed set. The constructor's $filter argument is not used with a filter by any shipped code path.
if (strtolower($val[1]) === 'like') {
$this->where .= $DB->sql_like($sqlfield, ' :' . $col, false, false);
} else {
$this->where .= $sqlfield . $val[1] . ' :' . $col . ' ';
}
Map the operator through an explicit allow-list instead of concatenating it verbatim:
$allowedops = ['>' => '>', '<' => '<', '>=' => '>=', '<=' => '<='];
$op = $allowedops[trim($val[1])] ?? null;
if ($op === null) {
throw new \InvalidArgumentException('Unsupported filter operator');
}
$this->where .= $sqlfield . ' ' . $op . ' :' . $col . ' ';
if (is_array($filter)) {
$this->filter = $filter;
}
Route the constructor filter through the validating setter so column names are always whitelisted:
if (is_array($filter)) {
$this->set_filter($filter);
}
The GitHub Actions workflow gates every static-analysis and validation step on if: ${{ cancelled() }}, which is true only when the workflow has been cancelled. As a result PHP Lint, PHP Mess Detector, Moodle Code Checker, PHPDoc Checker, plugin validation and upgrade-savepoint checks never run during normal CI. Only the PHPUnit step (if: ${{ !cancelled() }}) executes.
This silently disables the coding-standard, documentation and API-validation gates that would normally catch issues before release. The condition was almost certainly intended to be ${{ !cancelled() }} (or success()), matching the PHPUnit step.
Low risk. This is a CI/repository-hygiene issue with no runtime security impact. Its practical effect is that regressions in coding standards, PHPDoc and plugin validation are not detected automatically, increasing the chance of latent quality issues going unnoticed between releases.
The workflow otherwise builds a full Moodle site across PHP 8.2-8.4 and Moodle 4.05-5.02 with PostgreSQL/MariaDB and runs PHPUnit. Only the quality-gate steps are affected by the cancelled() condition.
- name: Moodle Code Checker
if: ${{ cancelled() }}
run: moodle-plugin-ci codechecker --max-warnings 0
Enable the quality gates by inverting the condition on each affected step (PHP Lint, PHP Mess Detector, Moodle Code Checker, PHPDoc Checker, Validating, Check upgrade savepoints):
- name: Moodle Code Checker
if: ${{ !cancelled() }}
run: moodle-plugin-ci codechecker --max-warnings 0
Strong security fundamentals. The plugin parameterises all database access through $DB placeholders, escapes report output with format_string() (HTML escaping on by default), constrains the CSV-download filename with PARAM_ALPHANUM (blocking path traversal), includes sesskey on the suspend/delete action links (with the actual state change performed and re-checked by core admin/user.php), and prefixes spreadsheet-formula characters in CSV cells. index.php enforces require_login() plus a system-context capability check, and both CLI scripts are protected from web execution by define('CLI_SCRIPT', true).
CSV/log file lifecycle. Generated CSV exports and debug logs are written to $CFG->tempdir/report_ldapaccounts/ and are never removed by the plugin; they persist until Moodle's temp-directory purge. Because they can contain personal data, consider deleting the CSV after download and rotating/limiting the debug log. A db/uninstall.php is not strictly required, since these files live in the self-cleaning temp directory and plugin settings are removed automatically on uninstall.
Content-Disposition header. In index.php the download filename is emitted as filename=ldapaccounts-"...csv" with mismatched quoting, producing a slightly malformed header/filename. This is cosmetic and has no security impact.
PHP 8.4 forward-compatibility. cli/ldapaccounts.php calls fputcsv($fp, $fields, $csvdelimiter) without the enclosure/escape arguments. PHP 8.4 is tightening CSV escaping semantics around the default escape value; passing the arguments explicitly (as user_table::output_csv() already does) avoids any future deprecation notice. Worth confirming against the target PHP version.