Skip to content

Commit 8b5779e

Browse files
committed
Harden plugin security: fix XSS, sanitize emails, secure state file
- Move API test state file to $CFG->dataroot (not web-accessible) - Remove raw API payload and response body from error notification emails (HTTP code and curl error are retained for debugging) - Escape all user-supplied output in log_table.php with s() - Escape JSON embedded in JS context with JSON_HEX_* flags - Replace var_export of raw API response in mtrace with safe message
1 parent b77990e commit 8b5779e

File tree

6 files changed

+11
-17
lines changed

6 files changed

+11
-17
lines changed

.gitignore

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
.test_enrolments_state.json
2-
.apitest_state.json
32
/vendor/
43
composer.lock

api-test.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@
4545
$apitesturl = get_config('local_psaelmsync', 'apiurl');
4646
$apitesttoken = get_config('local_psaelmsync', 'apitoken');
4747

48-
// State file for tracking enrolled users across runs.
49-
define('APITEST_STATE_FILE', __DIR__ . '/.apitest_state.json');
48+
// State file stored in Moodle's data directory (not web-accessible).
49+
define('APITEST_STATE_FILE', $CFG->dataroot . '/psaelmsync_apitest_state.json');
5050

5151
// Name pools for generating fake users.
5252
define('APITEST_FIRST_NAMES', [

classes/log_table.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ public function col_action($values) {
102102
* @return string The HTML link to the user profile.
103103
*/
104104
public function col_user_lastname($values) {
105-
$name = $values->user_firstname . ' ' . $values->user_lastname;
105+
$name = s($values->user_firstname . ' ' . $values->user_lastname);
106106
$userurl = new \moodle_url('/user/view.php', ['id' => $values->user_id]);
107107
return \html_writer::link($userurl, $name);
108108
}
@@ -114,7 +114,7 @@ public function col_user_lastname($values) {
114114
* @return string The user email address.
115115
*/
116116
public function col_user_email($values) {
117-
return $values->user_email;
117+
return s($values->user_email);
118118
}
119119

120120
/**
@@ -124,7 +124,7 @@ public function col_user_email($values) {
124124
* @return string The user GUID.
125125
*/
126126
public function col_user_guid($values) {
127-
return $values->user_guid;
127+
return s($values->user_guid);
128128
}
129129

130130
/**
@@ -135,7 +135,7 @@ public function col_user_guid($values) {
135135
*/
136136
public function col_course_name($values) {
137137
$courseurl = new \moodle_url('/user/index.php', ['id' => $values->course_id]);
138-
return \html_writer::link($courseurl, $values->course_name);
138+
return \html_writer::link($courseurl, s($values->course_name));
139139
}
140140

141141
/**
@@ -146,9 +146,9 @@ public function col_course_name($values) {
146146
*/
147147
public function col_user_details($values) {
148148
$userurl = new \moodle_url('/user/profile.php', ['id' => $values->user_id]);
149-
$userdetails = "{$values->user_firstname} {$values->user_lastname}"
150-
. "<br>GUID: {$values->user_guid}"
151-
. "<br>Email: {$values->user_email}";
149+
$userdetails = s($values->user_firstname . ' ' . $values->user_lastname)
150+
. "<br>GUID: " . s($values->user_guid)
151+
. "<br>Email: " . s($values->user_email);
152152
return \html_writer::link($userurl, $userdetails);
153153
}
154154

classes/observer.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,6 @@ private static function send_api_failure_notification(
311311
$message .= "API URL: " . $apiurl . "\n";
312312
$message .= "HTTP Code: " . $httpcode . "\n";
313313
$message .= "Error: " . $curlerror . "\n";
314-
$message .= "Payload: " . $jsondata . "\n";
315-
$message .= "Response Body: " . $responsebody . "\n";
316314

317315
$dummyuser = new \stdClass();
318316
$dummyuser->email = 'noreply-psalssync@learning.gww.gov.bc.ca';

dashboard-intake.php

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,14 @@
116116
$chartdata['errors'][] = $run->errorcount;
117117
}
118118

119-
// Encode the data for use in JavaScript.
120-
$chartdatajson = json_encode($chartdata);
121119
?>
122120

123121
<!-- Include Chart.js -->
124122
<script src="https://cdn.jsdelivr.net/npm/chart.js"></script>
125123
<script>
126124
document.addEventListener('DOMContentLoaded', function () {
127125
var ctx = document.getElementById('runsChart').getContext('2d');
128-
var chartData = <?php echo $chartdatajson; ?>;
126+
var chartData = <?php echo json_encode($chartdata, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT); ?>;
129127

130128
new Chart(ctx, {
131129
type: 'line',

lib.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,7 @@ function local_psaelmsync_sync() {
103103
$data = json_decode($response, true);
104104

105105
if (empty($data)) {
106-
mtrace('PSA Enrol Sync: No data received from API: '
107-
. var_export($response, true));
106+
mtrace('PSA Enrol Sync: No data received from API (empty or invalid JSON response).');
108107
return;
109108
}
110109

0 commit comments

Comments
 (0)