Fix CSRF in ajax.php enabling privilege escalation by requiring sessk… - #3391
Open
Patryk Mroczko (patmr7) wants to merge 1 commit into
Open
Fix CSRF in ajax.php enabling privilege escalation by requiring sessk…#3391Patryk Mroczko (patmr7) wants to merge 1 commit into
Patryk Mroczko (patmr7) wants to merge 1 commit into
Conversation
…ey validation local/o365/ajax.php required only require_login() and moodle/site:config, with no sesskey check on any of its state-changing modes. mode_checkteamsmoodlesetup in particular enables web services, the REST protocol, frame embedding, the bundled o365_webservices external service, and grants moodle/webservice:createtoken and webservice/rest:use to the default authenticated-user role - all unconditionally, on a simple GET request. A forged cross-site request against a logged-in administrator (e.g. via an <img> tag) silently expands these permissions to every authenticated user, regardless of whether the site uses Teams SSO at all. Add require_sesskey() to ajax.php, and append sesskey to the ajax.php URL built by the three admin_setting classes that call it (moodlesetup, verifysetup, serviceresource). These build their AJAX calls with plain jQuery rather than a sesskey-aware core JS module, so the sesskey is carried in the URL's query string instead, where jQuery's GET request serialization appends the remaining parameters.
There was a problem hiding this comment.
Pull request overview
This PR hardens the local_o365 admin AJAX endpoint against CSRF by enforcing Moodle sesskey validation and ensuring the admin-setting UI includes a sesskey when calling the endpoint.
Changes:
- Add
require_sesskey()tolocal/o365/ajax.phpto block CSRF against state-changing modes. - Append
sesskeyto the/local/o365/ajax.phpURL used by the three admin setting UI widgets (moodlesetup, verifysetup, serviceresource).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| local/o365/ajax.php | Enforces sesskey validation for all AJAX modes. |
| local/o365/classes/adminsetting/moodlesetup.php | Includes sesskey in the AJAX endpoint URL passed to the JS widget. |
| local/o365/classes/adminsetting/verifysetup.php | Includes sesskey in the AJAX endpoint URL passed to the JS widget. |
| local/o365/classes/adminsetting/serviceresource.php | Includes sesskey in the AJAX endpoint URL passed to the JS widget. |
Suppressed comments (1)
local/o365/classes/adminsetting/moodlesetup.php:99
- The JS widget (moodlesetup.js) references
opts.strerrorcheckwhen rendering errors, but this options object never sets that key. In error cases the UI will displayundefinedinstead of a localized error message.
$ajaxurl = new url('/local/o365/ajax.php', ['sesskey' => sesskey()]);
$settinghtml .= '<script>
$(function() {
var opts = {
url: "' . $ajaxurl->out() . '",
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
30
to
+33
| require_login(); | ||
| $mode = required_param('mode', PARAM_TEXT); | ||
| require_capability('moodle/site:config', system::instance()); | ||
| require_sesskey(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ey validation
local/o365/ajax.php required only require_login() and moodle/site:config, with no sesskey check on any of its state-changing modes. mode_checkteamsmoodlesetup in particular enables web services, the REST protocol, frame embedding, the bundled o365_webservices external service, and grants moodle/webservice:createtoken and webservice/rest:use to the default authenticated-user role - all unconditionally, on a simple GET request. A forged cross-site request against a logged-in administrator (e.g. via an![]()
tag) silently expands these permissions to every authenticated user, regardless of whether the site uses Teams SSO at all.
Add require_sesskey() to ajax.php, and append sesskey to the ajax.php URL built by the three admin_setting classes that call it (moodlesetup, verifysetup, serviceresource). These build their AJAX calls with plain jQuery rather than a sesskey-aware core JS module, so the sesskey is carried in the URL's query string instead, where jQuery's GET request serialization appends the remaining parameters.