From 136b543ecdf47b6cd6bf9cf071e3794411a05e7c Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 22 Oct 2026 21:03:51 -0600 Subject: [PATCH 1/2] fix: escape device fields in the Monitor list view Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + includes/render.php | 8 +- tests/Unit/MonitorListViewEscapingTest.php | 87 ++++++++++++++++++++++ 3 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/MonitorListViewEscapingTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 614b7fb..efd102a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ * security: Require POST for the save, remove, mute and card-order actions so csrf-magic covers them, and only let a dashboard's owner save over it * security: Bind the plugin version lookup and the device_remove cleanup deletes as prepared statements instead of building them from the device list +* bug: Escape the device description, location, site name and Monitor Text in the List view * bug: Restore alert audio for down and triggered devices; getMonitorSound() looked for sounds in includes/sounds/ after the controller moved, so the audio element was never rendered * feature: Replace the single Status column/filter with a multi-select Events filter (additive) and an Events column that shows a short pill per active event (Down, Recovering, Triggered, Down/Triggered M/A, Warn/Alert Ping, Failed Svc), 'None' when clean; the Status scope dropdown now only selects All Monitored / All Devices / Not Monitored / Without Thresholds * bug: Centre the tile hostname under the glyph and keep it clickable in the Tiles & Time view diff --git a/includes/render.php b/includes/render.php index 6870ec4..b053699 100644 --- a/includes/render.php +++ b/includes/render.php @@ -1377,7 +1377,7 @@ function renderHostList(array $host): string { } if ($host['status'] < 3 || $host['status'] == 5) { - $host_admin = $host['monitor_text']; + $host_admin = html_escape($host['monitor_text']); } else { $host_admin = ''; } @@ -1463,9 +1463,9 @@ function renderHostList(array $host): string { } form_selectable_cell($actions, $host['id'], '1%', 'left'); - form_selectable_cell($host['description'], $host['id'], '', 'left'); - form_selectable_cell($host['location'], $host['id'], '', 'left'); - form_selectable_cell($host['site_name'], $host['id'], '', 'left'); + form_selectable_cell(html_escape($host['description']), $host['id'], '', 'left'); + form_selectable_cell(html_escape($host['location']), $host['id'], '', 'left'); + form_selectable_cell(html_escape($host['site_name']), $host['id'], '', 'left'); form_selectable_cell($host['id'], $host['id'], '', 'left'); form_selectable_cell($host_crit, $host['id'], '', 'left'); form_selectable_cell($events_cell, $host['id'], '', 'center'); diff --git a/tests/Unit/MonitorListViewEscapingTest.php b/tests/Unit/MonitorListViewEscapingTest.php new file mode 100644 index 0000000..28ae90c --- /dev/null +++ b/tests/Unit/MonitorListViewEscapingTest.php @@ -0,0 +1,87 @@ +" . $contents . "\n"; + } + } + + if (!function_exists('form_end_row')) { + function form_end_row() { + print "\n"; + } + } + + if (!function_exists('get_timeinstate')) { + function get_timeinstate($host) { + return '1m'; + } + } + + require_once __DIR__ . '/../../includes/render.php'; +}); + +it('escapes device fields in the list view', function () { + global $criticalities, $iclasses, $thold_counts, $servcheck_counts; + + $criticalities = []; + $iclasses = []; + $thold_counts = []; + $servcheck_counts = []; + + $host = [ + 'id' => 7, + 'status' => 1, + 'status_rec_date' => '2026-01-01 00:00:00', + 'status_fail_date' => '2026-01-01 00:00:00', + 'description' => 'core & "edge"', + 'location' => 'rack & \'row\'', + 'site_name' => 'HQ & "annex"', + 'monitor_text' => 'note & "text"', + 'monitor_criticality' => 0, + 'availability_method' => 0, + 'availability' => 100, + ]; + + $output = renderHostList($host); + + expect($output)->toContain('<b>core</b> & "edge"'); + expect($output)->toContain('<i>rack</i> & 'row''); + expect($output)->toContain('<u>HQ</u> & "annex"'); + expect($output)->toContain('<s>note</s> & "text"'); + + foreach (['', '', '', ''] as $markup) { + expect($output)->not->toContain($markup); + } + + expect($output)->not->toContain('&amp;'); +}); From 90e9d2c594a7492ca35582e58f3953eba58d9cd7 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 24 Oct 2026 00:17:00 -0600 Subject: [PATCH 2/2] fix(monitor): use core escaped cells for device fields Signed-off-by: Thomas Vincent --- includes/render.php | 10 +++++----- tests/Unit/MonitorListViewEscapingTest.php | 23 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/includes/render.php b/includes/render.php index b053699..1e13c57 100644 --- a/includes/render.php +++ b/includes/render.php @@ -1377,7 +1377,7 @@ function renderHostList(array $host): string { } if ($host['status'] < 3 || $host['status'] == 5) { - $host_admin = html_escape($host['monitor_text']); + $host_admin = $host['monitor_text']; } else { $host_admin = ''; } @@ -1463,9 +1463,9 @@ function renderHostList(array $host): string { } form_selectable_cell($actions, $host['id'], '1%', 'left'); - form_selectable_cell(html_escape($host['description']), $host['id'], '', 'left'); - form_selectable_cell(html_escape($host['location']), $host['id'], '', 'left'); - form_selectable_cell(html_escape($host['site_name']), $host['id'], '', 'left'); + form_selectable_ecell($host['description'], $host['id'], '', 'left'); + form_selectable_ecell($host['location'], $host['id'], '', 'left'); + form_selectable_ecell($host['site_name'], $host['id'], '', 'left'); form_selectable_cell($host['id'], $host['id'], '', 'left'); form_selectable_cell($host_crit, $host['id'], '', 'left'); form_selectable_cell($events_cell, $host['id'], '', 'center'); @@ -1479,7 +1479,7 @@ function renderHostList(array $host): string { } form_selectable_cell($host_warn, $host['id'], '', 'left'); - form_selectable_cell($host_admin, $host['id'], '', 'white-space:pre-wrap;text-align:left'); + form_selectable_ecell($host_admin, $host['id'], '', 'white-space:pre-wrap;text-align:left'); form_selectable_cell(round($host['availability'], 2) . ' %', $host['id'], '', 'right'); form_selectable_cell($host_avg, $host['id'], '', 'right'); form_selectable_cell($dt, $host['id'], '', 'right'); diff --git a/tests/Unit/MonitorListViewEscapingTest.php b/tests/Unit/MonitorListViewEscapingTest.php index 28ae90c..7e4267f 100644 --- a/tests/Unit/MonitorListViewEscapingTest.php +++ b/tests/Unit/MonitorListViewEscapingTest.php @@ -24,14 +24,20 @@ /* * The List view hands device fields to form_selectable_cell(), which prints - * its argument as-is, so renderHostList() has to escape them itself. + * its argument as-is, so renderHostList() passes plain device fields through the core escaping cell renderer. */ beforeAll(function () { // Same output as Cacti's lib/html_utility.php versions of these helpers. if (!function_exists('form_selectable_cell')) { function form_selectable_cell($contents, $id, $width = '', $style_or_class = '', $title = '') { + $trace = debug_backtrace(0, 2); + if (($trace[1]['function'] ?? '') === 'form_selectable_ecell') { + $GLOBALS['monitor_test_ecell_values'][] = $trace[1]['args'][0]; + } print "\t" . $contents . "\n"; + + return true; } } @@ -47,6 +53,18 @@ function get_timeinstate($host) { } } + $source = file_get_contents($GLOBALS['config']['base_path'] . '/lib/html_utility.php'); + if (!preg_match('/^function form_selectable_ecell\([^\n]*\n.*?^}/ms', $source, $definition)) { + throw new RuntimeException('Cannot load the core escaping cell renderer'); + } + $path = tempnam(sys_get_temp_dir(), 'monitor-core-cell-'); + try { + file_put_contents($path, ' 100, ]; + $GLOBALS['monitor_test_ecell_values'] = []; $output = renderHostList($host); + expect($GLOBALS['monitor_test_ecell_values'])->toBe([$host['description'], $host['location'], $host['site_name'], $host['monitor_text']]); + expect($output)->toContain('<b>core</b> & "edge"'); expect($output)->toContain('<i>rack</i> & 'row''); expect($output)->toContain('<u>HQ</u> & "annex"');