Skip to content

Commit c95ea06

Browse files
somethingwithproofsomethingwithproof
committed
fix(security): escape request reuse in monitor views
1 parent bfd5a42 commit c95ea06

5 files changed

Lines changed: 108 additions & 8 deletions

File tree

‎monitor_controller.php‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,7 @@ function monitorRenderPrimaryFilterRow(array $dashboards, array $monitor_status,
451451
}
452452

453453
print '<input type="button" value="' . (get_request_var('mute') == 'false' ? getMuteText() : getUnmuteText()) . '" id="sound" title="' . (get_request_var('mute') == 'false' ? __('%s Alert for downed Devices', getMuteText(), 'monitor') : __('%s Alerts for downed Devices', getUnmuteText(), 'monitor')) . '">' . PHP_EOL;
454-
print '<input id="downhosts" type="hidden" value="' . get_request_var('downhosts') . '"><input id="mute" type="hidden" value="' . get_request_var('mute') . '">' . PHP_EOL;
454+
print '<input id="downhosts" type="hidden" value="' . html_escape(get_request_var('downhosts')) . '"><input id="mute" type="hidden" value="' . html_escape(get_request_var('mute')) . '">' . PHP_EOL;
455455
print '</span></td>';
456456
}
457457

@@ -549,23 +549,23 @@ function monitorRenderGroupingDropdowns(array $classes, array $criticalities, ar
549549
*/
550550
function monitorRenderHiddenFilterInputs(): void {
551551
if (get_request_var('grouping') != 'tree') {
552-
print '<td><input type="hidden" id="tree" value="' . get_request_var('tree') . '"></td>' . PHP_EOL;
552+
print '<td><input type="hidden" id="tree" value="' . html_escape(get_request_var('tree')) . '"></td>' . PHP_EOL;
553553
}
554554

555555
if (get_request_var('grouping') != 'site') {
556-
print '<td><input type="hidden" id="site" value="' . get_request_var('site') . '"></td>' . PHP_EOL;
556+
print '<td><input type="hidden" id="site" value="' . html_escape(get_request_var('site')) . '"></td>' . PHP_EOL;
557557
}
558558

559559
if (get_request_var('grouping') != 'template') {
560-
print '<td><input type="hidden" id="template" value="' . get_request_var('template') . '"></td>' . PHP_EOL;
560+
print '<td><input type="hidden" id="template" value="' . html_escape(get_request_var('template')) . '"></td>' . PHP_EOL;
561561
}
562562

563563
if (get_request_var('view') == 'list') {
564-
print '<td><input type="hidden" id="size" value="' . get_request_var('size') . '"></td>' . PHP_EOL;
564+
print '<td><input type="hidden" id="size" value="' . html_escape(get_request_var('size')) . '"></td>' . PHP_EOL;
565565
}
566566

567567
if (get_request_var('view') != 'default') {
568-
print '<td><input type="hidden" id="trim" value="' . get_request_var('trim') . '"></td>' . PHP_EOL;
568+
print '<td><input type="hidden" id="trim" value="' . html_escape(get_request_var('trim')) . '"></td>' . PHP_EOL;
569569
}
570570
}
571571

‎monitor_render.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -963,7 +963,7 @@ function renderHeaderList(int $total_rows = 0, int $rows = 0): string {
963963

964964
ob_start();
965965

966-
$nav = html_nav_bar('monitor.php?rfilter=' . get_request_var('rfilter'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, 12, __('Devices'), 'page', 'main');
966+
$nav = html_nav_bar('monitor.php?rfilter=' . rawurlencode(get_request_var('rfilter')), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, 12, __('Devices'), 'page', 'main');
967967

968968
html_start_box(__('Monitored Devices', 'monitor'), '100%', false, 3, 'center', '');
969969

@@ -1043,7 +1043,7 @@ function renderFooterList(int $total_rows, int $rows): string {
10431043
html_end_box(false);
10441044

10451045
if ($total_rows > 0) {
1046-
$nav = html_nav_bar('monitor.php?rfilter=' . get_request_var('rfilter'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, 12, __('Devices'), 'page', 'main');
1046+
$nav = html_nav_bar('monitor.php?rfilter=' . rawurlencode(get_request_var('rfilter')), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, 12, __('Devices'), 'page', 'main');
10471047

10481048
print $nav;
10491049
}

‎tests/Integration/test_monitor_request_output_wiring.php‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDTool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
$checks = array(
11+
__DIR__ . '/../../monitor_controller.php' => array(
12+
"html_escape(get_request_var('downhosts'))",
13+
"html_escape(get_request_var('mute'))",
14+
"html_escape(get_request_var('tree'))",
15+
"html_escape(get_request_var('site'))",
16+
"html_escape(get_request_var('template'))",
17+
"html_escape(get_request_var('size'))",
18+
"html_escape(get_request_var('trim'))",
19+
),
20+
__DIR__ . '/../../monitor_render.php' => array(
21+
"rawurlencode(get_request_var('rfilter'))",
22+
),
23+
);
24+
25+
foreach ($checks as $path => $patterns) {
26+
$contents = file_get_contents($path);
27+
28+
if ($contents === false) {
29+
fwrite(STDERR, "Unable to read {$path}\n");
30+
exit(1);
31+
}
32+
33+
foreach ($patterns as $pattern) {
34+
if (strpos($contents, $pattern) === false) {
35+
fwrite(STDERR, "Missing expected output hardening: {$pattern}\n");
36+
exit(1);
37+
}
38+
}
39+
}
40+
41+
print "OK\n";

‎tests/e2e/test_monitor_no_raw_request_reuse.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDTool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
$checks = array(
11+
__DIR__ . '/../../monitor_controller.php' => array(
12+
"get_request_var('downhosts') . '\"><input id=\"mute\" type=\"hidden\" value=\"' . get_request_var('mute')",
13+
"get_request_var('tree') . '\"></td>'",
14+
"get_request_var('site') . '\"></td>'",
15+
"get_request_var('template') . '\"></td>'",
16+
"get_request_var('size') . '\"></td>'",
17+
"get_request_var('trim') . '\"></td>'",
18+
),
19+
__DIR__ . '/../../monitor_render.php' => array(
20+
"monitor.php?rfilter=' . get_request_var('rfilter')",
21+
),
22+
);
23+
24+
foreach ($checks as $path => $patterns) {
25+
$contents = file_get_contents($path);
26+
27+
if ($contents === false) {
28+
fwrite(STDERR, "Unable to read {$path}\n");
29+
exit(1);
30+
}
31+
32+
foreach ($patterns as $pattern) {
33+
if (strpos($contents, $pattern) !== false) {
34+
fwrite(STDERR, "Raw request reuse remains: {$pattern}\n");
35+
exit(1);
36+
}
37+
}
38+
}
39+
40+
print "OK\n";

‎tests/unit/test_request_output_escaping.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
/*
3+
+-------------------------------------------------------------------------+
4+
| Copyright (C) 2004-2026 The Cacti Group |
5+
+-------------------------------------------------------------------------+
6+
| Cacti: The Complete RRDTool-based Graphing Solution |
7+
+-------------------------------------------------------------------------+
8+
*/
9+
10+
$payload = "\" autofocus onfocus=\"alert(1)";
11+
$escaped = htmlspecialchars($payload, ENT_QUOTES, 'UTF-8');
12+
13+
if (strpos($escaped, '"') === false && strpos($escaped, '&quot;') !== false) {
14+
print "OK\n";
15+
exit(0);
16+
}
17+
18+
fwrite(STDERR, "Expected request values to be escaped for hidden inputs\n");
19+
exit(1);

0 commit comments

Comments
 (0)