fix: require POST for monitor state changes and check dashboard ownership on save

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 --- 3.0 ---
 
+* 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
 * 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

includes/controller.php

@@ -93,6 +93,29 @@
 	return $owner !== null && $owner !== '' && (int) $owner === (int) ($_SESSION['sess_user_id'] ?? -1);
 }
 
+/**
+ * Requested action, or '' when a state-changing action arrives without POST.
+ *
+ * csrf-magic only checks POST requests, so these actions must not run from a
+ * GET or the token check never sees them.
+ *
+ * @return string
+ */
+function monitorRequestAction(): string {
+	$action = get_nfilter_request_var('action');
+
+	if (!is_string($action)) {
+		return '';
+	}
+
+	if (in_array($action, ['save', 'saveDb', 'remove', 'ajax_mute_all', 'ajax_unmute_all', 'ajax_saveorder'], true)
+		&& ($_SERVER['REQUEST_METHOD'] ?? '') !== 'POST') {
+		return '';
+	}
+
+	return $action;
+}
+
 /**
  * Persist a grouping's card ordering after a drag/drop reorder.
  *
@@ -975,12 +998,7 @@
 			'site'     => get_request_var('site'),
 		];
 
-		if (!isset_request_var('user')) {
-			$user = $_SESSION['sess_user_id'];
-		} else {
-			$user = get_request_var('user');
-		}
-
+		$user = $_SESSION['sess_user_id'];
 		$id   = get_request_var('dashboard');
 		$name = get_nfilter_request_var('name');
 
@@ -998,13 +1016,21 @@
 		$save['user_id']    = $user;
 		$save['properties'] = json_encode($props);
 
-		$id = sql_save($save, 'plugin_monitor_dashboards');
+		$level = MESSAGE_LEVEL_INFO;
+
+		// sql_save() updates an existing id in place, so only the owner may target one.
+		if ($id > 0 && !monitorDashboardOwnedByUser($id)) {
+			$id    = 0;
+			$level = MESSAGE_LEVEL_ERROR;
+		} else {
+			$id = sql_save($save, 'plugin_monitor_dashboards');
+		}
 
 		if (!empty($id)) {
 			raise_message('monitorsaved', __('Dashboard \'%s\' has been Saved!', $name, 'monitor'), MESSAGE_LEVEL_INFO);
 			set_request_var('dashboard', $id);
 		} else {
-			raise_message('monitornotsaved', __('Dashboard \'%s\' could not be Saved!', $name, 'monitor'), MESSAGE_LEVEL_INFO);
+			raise_message('monitornotsaved', __('Dashboard \'%s\' could not be Saved!', $name, 'monitor'), $level);
 			set_request_var('dashboard', '0');
 		}
 	}

js/monitor.js

@@ -182,7 +182,11 @@ function applyFilter(action = '') {
 		strURL += `&events=${events ? events.join(',') : ''}`;
 	}
 
-	loadIt(strURL);
+	if (action === 'ajax_mute_all' || action === 'ajax_unmute_all') {
+		postIt(strURL, { __csrf_magic: csrfMagicToken });
+	} else {
+		loadIt(strURL);
+	}
 }
 
 function saveFilter() {
@@ -242,8 +246,13 @@ function saveNewDashboard(action) {
 }
 
 function removeDashboard() {
-	const url = `monitor.php?action=remove&header=false&dashboard=${$('#dashboard').val()}`;
-	loadIt(url);
+	const post = {
+		action: 'remove',
+		dashboard: $('#dashboard').val(),
+		__csrf_magic: csrfMagicToken
+	};
+
+	postIt('monitor.php?header=false', post);
 }
 
 function loadIt(url) {

monitor.php

@@ -172,7 +172,7 @@
 	$servcheck_counts = monitorServcheckCounts();
 }
 
-switch (get_nfilter_request_var('action')) {
+switch (monitorRequestAction()) {
 	case 'ajax_status':
 		ajaxStatus();
 

tests/Unit/MonitorDashboardSaveTest.php

@@ -0,0 +1,85 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDTool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+require_once dirname(__DIR__, 2) . '/includes/controller.php';
+
+beforeEach(function () {
+	$GLOBALS['__test_db_calls'] = [];
+	$GLOBALS['__test_messages'] = [];
+	monitor_test_reset_db_mocks();
+	$_SESSION['sess_user_id'] = 5;
+});
+
+afterEach(function () {
+	unset($_SESSION['sess_user_id']);
+});
+
+function monitor_test_dashboard_saves(): array {
+	return array_values(array_filter($GLOBALS['__test_db_calls'], fn ($c) => $c['fn'] === 'sql_save'));
+}
+
+it('refuses to save over a dashboard owned by another user', function () {
+	monitor_test_mock_db('db_fetch_cell_prepared', 'SELECT user_id', '2');
+	test_set_request(['dashboard' => 7, 'name' => 'Mine']);
+
+	saveSettings();
+
+	expect(monitor_test_dashboard_saves())->toBe([])
+		->and(get_request_var('dashboard'))->toBe('0')
+		->and($GLOBALS['__test_messages'])->toBe([
+			['id' => 'monitornotsaved', 'text' => 'Dashboard \'%s\' could not be Saved!', 'level' => MESSAGE_LEVEL_ERROR],
+		]);
+});
+
+it('refuses to save over a shared dashboard', function () {
+	monitor_test_mock_db('db_fetch_cell_prepared', 'SELECT user_id', '0');
+	test_set_request(['dashboard' => 3, 'name' => 'Shared']);
+
+	saveSettings();
+
+	expect(monitor_test_dashboard_saves())->toBe([]);
+});
+
+it('saves a dashboard the user owns', function () {
+	monitor_test_mock_db('db_fetch_cell_prepared', 'SELECT user_id', '5');
+	test_set_request(['dashboard' => 7, 'name' => 'Mine']);
+
+	saveSettings();
+
+	$saves = monitor_test_dashboard_saves();
+
+	expect($saves)->toHaveCount(1)
+		->and($saves[0]['table'])->toBe('plugin_monitor_dashboards')
+		->and($saves[0]['data']['id'])->toBe(7)
+		->and($saves[0]['data']['user_id'])->toBe(5)
+		->and(array_column($GLOBALS['__test_messages'], 'level'))->toBe([MESSAGE_LEVEL_INFO]);
+});
+
+it('ignores a forged user when creating a dashboard', function () {
+	test_set_request(['dashboard' => -1, 'name' => 'New', 'user' => 2]);
+
+	saveSettings();
+
+	$saves = monitor_test_dashboard_saves();
+
+	expect($saves)->toHaveCount(1)
+		->and($saves[0]['data']['user_id'])->toBe(5);
+});
+
+it('ignores a forged user when updating an owned dashboard', function () {
+	monitor_test_mock_db('db_fetch_cell_prepared', 'SELECT user_id', '5');
+	test_set_request(['dashboard' => 7, 'name' => 'Mine', 'user' => 0]);
+
+	saveSettings();
+
+	$saves = monitor_test_dashboard_saves();
+
+	expect($saves)->toHaveCount(1)
+		->and($saves[0]['data']['user_id'])->toBe(5);
+});

tests/Unit/MonitorRequestActionTest.php

@@ -0,0 +1,64 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDTool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+require_once dirname(__DIR__, 2) . '/includes/controller.php';
+
+dataset('state_changing_actions', [
+	'save',
+	'saveDb',
+	'remove',
+	'ajax_mute_all',
+	'ajax_unmute_all',
+	'ajax_saveorder',
+]);
+
+afterEach(function () {
+	unset($_SERVER['REQUEST_METHOD']);
+});
+
+it('drops a state-changing action sent by GET', function (string $action) {
+	$_SERVER['REQUEST_METHOD'] = 'GET';
+	test_set_request(['action' => $action]);
+
+	expect(monitorRequestAction())->toBe('');
+})->with('state_changing_actions');
+
+it('drops a state-changing action when there is no request method', function (string $action) {
+	test_set_request(['action' => $action]);
+
+	expect(monitorRequestAction())->toBe('');
+})->with('state_changing_actions');
+
+it('dispatches a state-changing action sent by POST', function (string $action) {
+	$_SERVER['REQUEST_METHOD'] = 'POST';
+	test_set_request(['action' => $action]);
+
+	expect(monitorRequestAction())->toBe($action);
+})->with('state_changing_actions');
+
+it('still dispatches read-only actions sent by GET', function () {
+	$_SERVER['REQUEST_METHOD'] = 'GET';
+	test_set_request(['action' => 'ajax_status']);
+
+	expect(monitorRequestAction())->toBe('ajax_status');
+});
+
+it('ignores an action that is not a string', function () {
+	$_SERVER['REQUEST_METHOD'] = 'POST';
+	test_set_request(['action' => ['remove']]);
+
+	expect(monitorRequestAction())->toBe('');
+});
+
+it('routes the monitor.php dispatcher through the method check', function () {
+	$contents = file_get_contents(dirname(__DIR__, 2) . '/monitor.php');
+
+	expect($contents)->toContain('switch (monitorRequestAction()) {')
+		->and($contents)->not->toContain("switch (get_nfilter_request_var('action'))");
+});

tests/bootstrap-unit.php

@@ -377,6 +377,7 @@ function is_realm_allowed($realm) {
 
 if (!function_exists('raise_message')) {
 	function raise_message($id, $text = '', $level = 0) {
+		$GLOBALS['__test_messages'][] = array('id' => $id, 'text' => $text, 'level' => $level);
 	}
 }
 
@@ -386,6 +387,17 @@ function get_request_var($name) {
 	}
 }
 
+if (!function_exists('isset_request_var')) {
+	function isset_request_var($name) {
+		return isset($GLOBALS['__test_request'][$name]);
+	}
+}
+
+if (!function_exists('validate_store_request_vars')) {
+	function validate_store_request_vars($filters, $sess_prefix = '') {
+	}
+}
+
 if (!function_exists('get_nfilter_request_var')) {
 	function get_nfilter_request_var($name) {
 		return get_request_var($name);
@@ -433,6 +445,8 @@ function is_error_message() {
 
 if (!function_exists('sql_save')) {
 	function sql_save($array, $table, $key = 'id') {
+		$GLOBALS['__test_db_calls'][] = array('fn' => 'sql_save', 'table' => $table, 'data' => $array);
+
 		return isset($array['id']) ? $array['id'] : 1;
 	}
 }
@@ -457,8 +471,16 @@ function sql_save($array, $table, $key = 'id') {
 	define('POLLER_VERBOSITY_NONE', 6);
 }
 
+if (!defined('FILTER_VALIDATE_IS_REGEX')) {
+	define('FILTER_VALIDATE_IS_REGEX', 99999);
+}
+
+if (!defined('MESSAGE_LEVEL_INFO')) {
+	define('MESSAGE_LEVEL_INFO', 1);
+}
+
 if (!defined('MESSAGE_LEVEL_ERROR')) {
-	define('MESSAGE_LEVEL_ERROR', 1);
+	define('MESSAGE_LEVEL_ERROR', 3);
 }
 
 if (!function_exists('plugin_test_read_source')) {