From 75e4ad639da2c4737c8f5c142b6ec883983d6187 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Tue, 10 Mar 2026 13:51:15 -0700 Subject: [PATCH 1/3] hardening: enforce POST+CSRF for purge syslog devices utility Refs #259 Signed-off-by: Thomas Vincent --- setup.php | 64 ++++++++++- tests/regression/issue259_csrf_purge_test.php | 106 ++++++++++++++++++ 2 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 tests/regression/issue259_csrf_purge_test.php diff --git a/setup.php b/setup.php index d1e7d8d..2e31b8a 100644 --- a/setup.php +++ b/setup.php @@ -1608,6 +1608,25 @@ function syslog_utilities_action($action) { } if ($action == 'purge_syslog_hosts') { + if ($_SERVER['REQUEST_METHOD'] !== 'POST') { + raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); + header('Location: utilities.php'); + exit; + } + + if (function_exists('csrf_check')) { + if (!csrf_check(false)) { + raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); + header('Location: utilities.php'); + exit; + } + } else { + cacti_log('WARNING: syslog purge blocked -- CSRF validation unavailable', false, 'SYSLOG'); + raise_message('syslog_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR); + header('Location: utilities.php'); + exit; + } + $records = 0; syslog_db_execute('DELETE FROM syslog_hosts @@ -1660,7 +1679,50 @@ function syslog_utilities_list() { - + '> + + diff --git a/tests/regression/issue259_csrf_purge_test.php b/tests/regression/issue259_csrf_purge_test.php new file mode 100644 index 0000000..54bdac2 --- /dev/null +++ b/tests/regression/issue259_csrf_purge_test.php @@ -0,0 +1,106 @@ + breakout in HTML script context +if (strpos($setup, 'JSON_HEX_TAG') === false) { + fwrite(STDERR, "json_encode() must use JSON_HEX_TAG to prevent script-context breakout.\n"); + exit(1); +} + +if (strpos($setup, 'JSON_HEX_AMP') === false) { + fwrite(STDERR, "json_encode() must use JSON_HEX_AMP to escape ampersands in script context.\n"); + exit(1); +} + +if (strpos($setup, 'JSON_HEX_APOS') === false) { + fwrite(STDERR, "json_encode() must use JSON_HEX_APOS.\n"); + exit(1); +} + +if (strpos($setup, 'JSON_HEX_QUOT') === false) { + fwrite(STDERR, "json_encode() must use JSON_HEX_QUOT.\n"); + exit(1); +} + +// Verify user-facing message does not expose CSRF internals (log message may use "CSRF") +if (strpos($setup, "raise_message('syslog_error', __('CSRF") !== false) { + fwrite(STDERR, "User-facing raise_message must not expose CSRF internals to end users.\n"); + exit(1); +} + +// Verify generic user-facing message is present +if (strpos($setup, "Invalid request. Please try again.") === false) { + fwrite(STDERR, "Fail-closed branch must use generic 'Invalid request. Please try again.' message.\n"); + exit(1); +} + +// Verify fail-closed raise_message uses MESSAGE_LEVEL_ERROR severity +if (strpos($setup, "raise_message('syslog_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR)") === false) { + fwrite(STDERR, "Fail-closed branch raise_message must use MESSAGE_LEVEL_ERROR severity.\n"); + exit(1); +} + +// Verify log message does not expose internal function name +if (strpos($setup, 'csrf_check() unavailable') !== false) { + fwrite(STDERR, "Log message must not name internal validation function.\n"); + exit(1); +} + +echo "issue259_csrf_purge_test passed\n"; From 5fe348adb8f2913a5cbe9b43512bc3eed18a9182 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 11 Apr 2026 03:59:17 -0700 Subject: [PATCH 2/3] fix(csrf): distinct error codes, audit log on non-POST, honest lint header - Distinct raise_message IDs per failure mode (syslog_method_error, syslog_csrf_error, syslog_csrf_unavailable) so log triage can differentiate non-POST, invalid token, and missing-helper paths - Add cacti_log entry on the non-POST rejection path so the audit trail is symmetric with the other two fail-closed branches - Document csrf_check($fatal=false) arg semantics inline so future readers see the helper contract - Rename the regression test comment block to call out explicitly that it is a source-scan lint, not a behavioral test; flag follow-up for real behavioral coverage once a DB-backed test harness exists Signed-off-by: Thomas Vincent --- setup.php | 11 +++++++--- tests/regression/issue259_csrf_purge_test.php | 20 ++++++++++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/setup.php b/setup.php index 2e31b8a..9d5df16 100644 --- a/setup.php +++ b/setup.php @@ -1609,20 +1609,25 @@ function syslog_utilities_action($action) { if ($action == 'purge_syslog_hosts') { if ($_SERVER['REQUEST_METHOD'] !== 'POST') { - raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); + cacti_log('WARNING: syslog purge blocked -- non-POST request', false, 'SYSLOG'); + raise_message('syslog_method_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); header('Location: utilities.php'); exit; } + // csrf_check($fatal) returns bool; $fatal=false tells the helper not to + // die/exit on failure so we can log and redirect with a user-visible + // message ourselves. if (function_exists('csrf_check')) { if (!csrf_check(false)) { - raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); + cacti_log('WARNING: syslog purge blocked -- CSRF token validation failed', false, 'SYSLOG'); + raise_message('syslog_csrf_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); header('Location: utilities.php'); exit; } } else { cacti_log('WARNING: syslog purge blocked -- CSRF validation unavailable', false, 'SYSLOG'); - raise_message('syslog_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR); + raise_message('syslog_csrf_unavailable', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR); header('Location: utilities.php'); exit; } diff --git a/tests/regression/issue259_csrf_purge_test.php b/tests/regression/issue259_csrf_purge_test.php index 54bdac2..dbcaa46 100644 --- a/tests/regression/issue259_csrf_purge_test.php +++ b/tests/regression/issue259_csrf_purge_test.php @@ -1,4 +1,18 @@ Date: Sat, 11 Apr 2026 13:37:45 -0700 Subject: [PATCH 3/3] fix(security): harden syslog bulk form and nav encoding --- syslog.php | 10 ++-- syslog_alerts.php | 4 +- syslog_removal.php | 4 +- syslog_reports.php | 4 +- ...sue279_bulk_form_and_nav_encoding_test.php | 58 +++++++++++++++++++ 5 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 tests/regression/issue279_bulk_form_and_nav_encoding_test.php diff --git a/syslog.php b/syslog.php index 0d1c45c..02c397a 100644 --- a/syslog.php +++ b/syslog.php @@ -1184,11 +1184,11 @@ function syslog_filter($sql_where, $tab) { ?> - + $save_html "; @@ -856,7 +856,7 @@ function syslog_alerts() { 'user' => [__('By User', 'syslog'), 'DESC'] ]; - $nav = html_nav_bar('syslog_alerts.php?filter=' . get_request_var('filter'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, cacti_sizeof($display_text) + 1, __('Alerts', 'syslog'), 'page', 'main'); + $nav = html_nav_bar('syslog_alerts.php?filter=' . rawurlencode(get_request_var('filter')), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, cacti_sizeof($display_text) + 1, __('Alerts', 'syslog'), 'page', 'main'); form_start('syslog_alerts.php', 'chk'); diff --git a/syslog_removal.php b/syslog_removal.php index b92193e..e0b7edd 100644 --- a/syslog_removal.php +++ b/syslog_removal.php @@ -234,7 +234,7 @@ function form_actions() { - + $save_html "; @@ -667,7 +667,7 @@ function syslog_removal() { form_start('syslog_removal.php', 'chk'); - $nav = html_nav_bar('syslog_removal.php?filter=' . get_request_var('filter'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, cacti_sizeof($display_text) + 1, __('Rules', 'syslog'), 'page', 'main'); + $nav = html_nav_bar('syslog_removal.php?filter=' . rawurlencode(get_request_var('filter')), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, cacti_sizeof($display_text) + 1, __('Rules', 'syslog'), 'page', 'main'); print $nav; diff --git a/syslog_reports.php b/syslog_reports.php index 048fbc8..b923efd 100644 --- a/syslog_reports.php +++ b/syslog_reports.php @@ -206,7 +206,7 @@ function form_actions() { - + $save_html \n"; @@ -704,7 +704,7 @@ function syslog_report() { 'user' => [__('By User', 'syslog'), 'DESC'] ]; - $nav = html_nav_bar('syslog_reports.php?filter=' . get_request_var('filter'), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, cacti_sizeof($display_text) + 1, __('Reports', 'syslog'), 'page', 'main'); + $nav = html_nav_bar('syslog_reports.php?filter=' . rawurlencode(get_request_var('filter')), MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, cacti_sizeof($display_text) + 1, __('Reports', 'syslog'), 'page', 'main'); form_start('syslog_reports.php', 'chk'); diff --git a/tests/regression/issue279_bulk_form_and_nav_encoding_test.php b/tests/regression/issue279_bulk_form_and_nav_encoding_test.php new file mode 100644 index 0000000..5af309f --- /dev/null +++ b/tests/regression/issue279_bulk_form_and_nav_encoding_test.php @@ -0,0 +1,58 @@ + file_get_contents(__DIR__ . '/../../syslog_removal.php'), + 'syslog_alerts.php' => file_get_contents(__DIR__ . '/../../syslog_alerts.php'), + 'syslog_reports.php' => file_get_contents(__DIR__ . '/../../syslog_reports.php'), + 'syslog.php' => file_get_contents(__DIR__ . '/../../syslog.php'), +); + +foreach ($targets as $file => $contents) { + if ($contents === false) { + fwrite(STDERR, "Unable to read $file\n"); + exit(1); + } +} + +foreach (array('syslog_removal.php', 'syslog_alerts.php', 'syslog_reports.php') as $file) { + if (strpos($targets[$file], "html_escape(get_request_var('drp_action'))") === false) { + fwrite(STDERR, "Expected escaped drp_action hidden field in $file\n"); + exit(1); + } + + if (strpos($targets[$file], "rawurlencode(get_request_var('filter'))") === false) { + fwrite(STDERR, "Expected URL-encoded filter nav value in $file\n"); + exit(1); + } + + if (strpos($targets[$file], "") !== false) { + fwrite(STDERR, "Legacy raw drp_action hidden field remains in $file\n"); + exit(1); + } +} + +$syslog = $targets['syslog.php']; + +if (strpos($syslog, "pageTab: ,") === false) { + fwrite(STDERR, "Expected JSON-encoded syslog pageTab value\n"); + exit(1); +} + +foreach (array( + "json_encode(__('Enter a search term', 'syslog'), JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT)", + "json_encode(__('Select Device(s)', 'syslog'), JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT)", + "json_encode(__('Devices Selected', 'syslog'), JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT)", + "json_encode(__('All Devices Selected', 'syslog'), JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT)", +) as $needle) { + if (strpos($syslog, $needle) === false) { + fwrite(STDERR, "Expected JS-safe initSyslogMain text encoding\n"); + exit(1); + } +} + +if (strpos($syslog, "pageTab: ''") !== false) { + fwrite(STDERR, "Legacy raw pageTab JS assignment still present\n"); + exit(1); +} + +echo "OK\n";