From aa000956f445e10485d2e98853a016e7c9626c35 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Tue, 10 Mar 2026 13:51:15 -0700 Subject: [PATCH 01/19] 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 46d0333d..0f356675 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 00000000..54bdac22 --- /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 9bc39fc289079c6064e9c5029114dc2743da02e4 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 11 Apr 2026 03:59:17 -0700 Subject: [PATCH 02/19] 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 0f356675..66e7ed69 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 54bdac22..dbcaa46f 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 03/19] fix(security): harden syslog bulk form and nav encoding Signed-off-by: Thomas Vincent --- 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 0d1c45c9..02c397a3 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 e047e68c..b8768b5a 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 fd7b2120..ecf2ef16 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 00000000..5af309f7 --- /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"; From 5a120cd9ea602b9d6311bc24fa90793cd11fae9c Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 4 Jun 2026 11:56:33 -0400 Subject: [PATCH 04/19] Update syslog_alerts.php Signed-off-by: Thomas Vincent --- syslog_alerts.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syslog_alerts.php b/syslog_alerts.php index b97f8c93..4e155dd1 100644 --- a/syslog_alerts.php +++ b/syslog_alerts.php @@ -856,7 +856,7 @@ function syslog_alerts() { 'user' => [__('By User', 'syslog'), 'DESC'] ]; - $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'); + $nav = html_nav_bar('syslog_alerts.php', 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'); From f22cfc4815ecf6bb36a01ec0aecac4066814abd2 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 4 Jun 2026 11:59:10 -0400 Subject: [PATCH 05/19] Refactor navigation bar URL in syslog_removal.php Signed-off-by: Thomas Vincent --- syslog_removal.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syslog_removal.php b/syslog_removal.php index b8768b5a..15ff88ef 100644 --- a/syslog_removal.php +++ b/syslog_removal.php @@ -667,7 +667,7 @@ function syslog_removal() { form_start('syslog_removal.php', 'chk'); - $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'); + $nav = html_nav_bar('syslog_removal.php', MAX_DISPLAY_PAGES, get_request_var('page'), $rows, $total_rows, cacti_sizeof($display_text) + 1, __('Rules', 'syslog'), 'page', 'main'); print $nav; From 93fca4cb809608d3866828b27d12ad0e449e4185 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Thu, 4 Jun 2026 11:59:50 -0400 Subject: [PATCH 06/19] Fix navigation bar URL in syslog_reports.php Signed-off-by: Thomas Vincent --- syslog_reports.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syslog_reports.php b/syslog_reports.php index ecf2ef16..f51e3f93 100644 --- a/syslog_reports.php +++ b/syslog_reports.php @@ -704,7 +704,7 @@ function syslog_report() { 'user' => [__('By User', 'syslog'), 'DESC'] ]; - $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'); + $nav = html_nav_bar('syslog_reports.php', 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'); From 330497b301d685f5ed3ba7e7128a3dac471d2249 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 13 Jul 2026 20:14:19 -0700 Subject: [PATCH 07/19] Address Syslog purge security review Signed-off-by: Thomas Vincent --- functions.php | 4 ++++ setup.php | 4 ++-- syslog.php | 10 +++++----- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/functions.php b/functions.php index 1e54c132..2a147b7c 100644 --- a/functions.php +++ b/functions.php @@ -47,6 +47,10 @@ function syslog_include_js() { Date: Mon, 13 Jul 2026 20:18:14 -0700 Subject: [PATCH 08/19] Preserve Syslog filter navigation safely Signed-off-by: Thomas Vincent --- syslog_alerts.php | 2 +- syslog_removal.php | 2 +- syslog_reports.php | 2 +- .../issue279_bulk_form_and_nav_encoding_test.php | 10 +++++----- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/syslog_alerts.php b/syslog_alerts.php index 4e155dd1..b97f8c93 100644 --- a/syslog_alerts.php +++ b/syslog_alerts.php @@ -856,7 +856,7 @@ function syslog_alerts() { 'user' => [__('By User', 'syslog'), 'DESC'] ]; - $nav = html_nav_bar('syslog_alerts.php', 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 15ff88ef..b8768b5a 100644 --- a/syslog_removal.php +++ b/syslog_removal.php @@ -667,7 +667,7 @@ function syslog_removal() { form_start('syslog_removal.php', 'chk'); - $nav = html_nav_bar('syslog_removal.php', 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 f51e3f93..ecf2ef16 100644 --- a/syslog_reports.php +++ b/syslog_reports.php @@ -704,7 +704,7 @@ function syslog_report() { 'user' => [__('By User', 'syslog'), 'DESC'] ]; - $nav = html_nav_bar('syslog_reports.php', 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 index 5af309f7..63f8e232 100644 --- a/tests/regression/issue279_bulk_form_and_nav_encoding_test.php +++ b/tests/regression/issue279_bulk_form_and_nav_encoding_test.php @@ -33,16 +33,16 @@ $syslog = $targets['syslog.php']; -if (strpos($syslog, "pageTab: ,") === false) { +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)", + "syslog_json_encode_for_script(__('Enter a search term', 'syslog'))", + "syslog_json_encode_for_script(__('Select Device(s)', 'syslog'))", + "syslog_json_encode_for_script(__('Devices Selected', 'syslog'))", + "syslog_json_encode_for_script(__('All Devices Selected', 'syslog'))", ) as $needle) { if (strpos($syslog, $needle) === false) { fwrite(STDERR, "Expected JS-safe initSyslogMain text encoding\n"); From 3837c56395ca94cfb5ed881018f1a3c69213616a Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 13 Jul 2026 22:04:27 -0700 Subject: [PATCH 09/19] Strengthen Syslog CSRF regression coverage Signed-off-by: Thomas Vincent --- tests/regression/issue259_csrf_purge_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/regression/issue259_csrf_purge_test.php b/tests/regression/issue259_csrf_purge_test.php index dbcaa46f..b2d30c07 100644 --- a/tests/regression/issue259_csrf_purge_test.php +++ b/tests/regression/issue259_csrf_purge_test.php @@ -94,7 +94,7 @@ } // Verify user-facing messages do not expose CSRF internals (log messages may use "CSRF") -if (preg_match("/raise_message\\('syslog_[a-z_]*', __\\('CSRF/", $setup)) { +if (preg_match('/raise_message\\s*\\(\\s*[^,]+,\\s*__\\(\\s*([\'\"])[^\'\"]*CSRF[^\'\"]*\\1/si', $setup)) { fwrite(STDERR, "User-facing raise_message must not expose CSRF internals to end users.\n"); exit(1); } From ba7728840b544327e59f8e257d48321f4de65581 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Tue, 14 Jul 2026 02:36:00 -0700 Subject: [PATCH 10/19] Use runner PHP without versioned Apache package Signed-off-by: Thomas Vincent --- .github/workflows/plugin-ci-workflow.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 86237dcf..3840fa29 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -143,7 +143,7 @@ jobs: run: sudo apt-get update - name: Install System Dependencies - run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php${{ matrix.php }} + run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping - name: Start SNMPD Agent and Test run: | From fb741434c2779be03f199f0d0b886fc7f994f8b8 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Wed, 29 Jul 2026 17:51:34 -0700 Subject: [PATCH 11/19] refactor: reuse syslog JSON script encoder Signed-off-by: Thomas Vincent --- setup.php | 6 ++--- tests/regression/issue259_csrf_purge_test.php | 25 ++++++++++++------- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/setup.php b/setup.php index f31eeba9..38197cbe 100644 --- a/setup.php +++ b/setup.php @@ -1692,21 +1692,21 @@ function syslog_utilities_list() { $(function() { $('#syslog_purge_hosts').on('click', function() { $('#syslog_purge_dialog').dialog({ - title: , + title: , minHeight: 80, minWidth: 400, resizable: false, draggable: true, buttons: { 'Cancel': { - text: , + text: , id: 'btnPurgeCancel', click: function() { $(this).dialog('close'); } }, 'Continue': { - text: , + text: , id: 'btnPurgeContinue', click: function() { $(this).dialog('close'); diff --git a/tests/regression/issue259_csrf_purge_test.php b/tests/regression/issue259_csrf_purge_test.php index b2d30c07..9d459d8c 100644 --- a/tests/regression/issue259_csrf_purge_test.php +++ b/tests/regression/issue259_csrf_purge_test.php @@ -14,10 +14,11 @@ * real behavioral coverage once a DB-backed test harness exists. */ -$setup = file_get_contents(dirname(__DIR__, 2) . '/setup.php'); +$setup = file_get_contents(dirname(__DIR__, 2) . '/setup.php'); +$functions = file_get_contents(dirname(__DIR__, 2) . '/functions.php'); -if ($setup === false) { - fwrite(STDERR, "Failed to read setup.php\n"); +if ($setup === false || $functions === false) { + fwrite(STDERR, "Failed to read setup.php or functions.php\n"); exit(1); } @@ -67,28 +68,34 @@ exit(1); } -if (strpos($setup, 'json_encode(__(') === false) { - fwrite(STDERR, "Expected json_encode(__(...)) for JS-safe encoding of confirm message.\n"); +if (strpos($setup, "syslog_json_encode_for_script(__('Confirm Purge', 'syslog'))") === false) { + fwrite(STDERR, "Expected syslog_json_encode_for_script() for JS-safe dialog title encoding.\n"); + exit(1); +} + +if (strpos($setup, "syslog_json_encode_for_script(__('Cancel', 'syslog'))") === false || + strpos($setup, "syslog_json_encode_for_script(__('Continue', 'syslog'))") === false) { + fwrite(STDERR, "Expected syslog_json_encode_for_script() for JS-safe button text encoding.\n"); exit(1); } // Verify json_encode uses JSON_HEX_TAG to prevent breakout in HTML script context -if (strpos($setup, 'JSON_HEX_TAG') === false) { +if (strpos($functions, '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) { +if (strpos($functions, '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) { +if (strpos($functions, 'JSON_HEX_APOS') === false) { fwrite(STDERR, "json_encode() must use JSON_HEX_APOS.\n"); exit(1); } -if (strpos($setup, 'JSON_HEX_QUOT') === false) { +if (strpos($functions, 'JSON_HEX_QUOT') === false) { fwrite(STDERR, "json_encode() must use JSON_HEX_QUOT.\n"); exit(1); } From 62b7576ef675e2469dfdc8f5cbcde37314264164 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 00:25:32 -0700 Subject: [PATCH 12/19] Modernize CSRF hardening for PHP 8.0 Signed-off-by: Thomas Vincent --- functions.php | 4 +-- setup.php | 22 ++++++------ tests/regression/issue259_csrf_purge_test.php | 36 +++++++++---------- ...sue279_bulk_form_and_nav_encoding_test.php | 22 ++++++------ 4 files changed, 42 insertions(+), 42 deletions(-) diff --git a/functions.php b/functions.php index 2a147b7c..d39c4352 100644 --- a/functions.php +++ b/functions.php @@ -47,8 +47,8 @@ function syslog_include_js() { breakout in HTML script context -if (strpos($functions, 'JSON_HEX_TAG') === false) { +if (!str_contains($functions, 'JSON_HEX_TAG')) { fwrite(STDERR, "json_encode() must use JSON_HEX_TAG to prevent script-context breakout.\n"); exit(1); } -if (strpos($functions, 'JSON_HEX_AMP') === false) { +if (!str_contains($functions, 'JSON_HEX_AMP')) { fwrite(STDERR, "json_encode() must use JSON_HEX_AMP to escape ampersands in script context.\n"); exit(1); } -if (strpos($functions, 'JSON_HEX_APOS') === false) { +if (!str_contains($functions, 'JSON_HEX_APOS')) { fwrite(STDERR, "json_encode() must use JSON_HEX_APOS.\n"); exit(1); } -if (strpos($functions, 'JSON_HEX_QUOT') === false) { +if (!str_contains($functions, 'JSON_HEX_QUOT')) { fwrite(STDERR, "json_encode() must use JSON_HEX_QUOT.\n"); exit(1); } @@ -107,19 +107,19 @@ } // Verify generic user-facing message is present -if (strpos($setup, "Invalid request. Please try again.") === false) { +if (!str_contains($setup, 'Invalid request. Please try again.')) { 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_csrf_unavailable', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR)") === false) { +if (!str_contains($setup, "raise_message('syslog_csrf_unavailable', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR)")) { 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) { +if (str_contains($setup, 'csrf_check() unavailable')) { fwrite(STDERR, "Log message must not name internal validation function.\n"); exit(1); } diff --git a/tests/regression/issue279_bulk_form_and_nav_encoding_test.php b/tests/regression/issue279_bulk_form_and_nav_encoding_test.php index 63f8e232..48f533c1 100644 --- a/tests/regression/issue279_bulk_form_and_nav_encoding_test.php +++ b/tests/regression/issue279_bulk_form_and_nav_encoding_test.php @@ -1,11 +1,11 @@ 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) { @@ -14,18 +14,18 @@ } } -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) { +foreach (['syslog_removal.php', 'syslog_alerts.php', 'syslog_reports.php'] as $file) { + if (!str_contains($targets[$file], "html_escape(get_request_var('drp_action'))")) { fwrite(STDERR, "Expected escaped drp_action hidden field in $file\n"); exit(1); } - if (strpos($targets[$file], "rawurlencode(get_request_var('filter'))") === false) { + if (!str_contains($targets[$file], "rawurlencode(get_request_var('filter'))")) { fwrite(STDERR, "Expected URL-encoded filter nav value in $file\n"); exit(1); } - if (strpos($targets[$file], "") !== false) { + if (str_contains($targets[$file], "")) { fwrite(STDERR, "Legacy raw drp_action hidden field remains in $file\n"); exit(1); } @@ -33,24 +33,24 @@ $syslog = $targets['syslog.php']; -if (strpos($syslog, "pageTab: ,") === false) { +if (!str_contains($syslog, "pageTab: ,")) { fwrite(STDERR, "Expected JSON-encoded syslog pageTab value\n"); exit(1); } -foreach (array( +foreach ([ "syslog_json_encode_for_script(__('Enter a search term', 'syslog'))", "syslog_json_encode_for_script(__('Select Device(s)', 'syslog'))", "syslog_json_encode_for_script(__('Devices Selected', 'syslog'))", "syslog_json_encode_for_script(__('All Devices Selected', 'syslog'))", -) as $needle) { - if (strpos($syslog, $needle) === false) { +] as $needle) { + if (!str_contains($syslog, $needle)) { fwrite(STDERR, "Expected JS-safe initSyslogMain text encoding\n"); exit(1); } } -if (strpos($syslog, "pageTab: ''") !== false) { +if (str_contains($syslog, "pageTab: ''")) { fwrite(STDERR, "Legacy raw pageTab JS assignment still present\n"); exit(1); } From 075e07766cd7d464909ad2e66e352d6f66640771 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 23:17:31 -0700 Subject: [PATCH 13/19] ci: repair the integration workflow Three defects, all failing open or failing at setup: - the plugin syntax check redirected find's own output rather than php's, so PHP errors never reached the grep testing for them; the step could not fail - MYSQL_AUTH_USR carried a literal tilde, because parameter expansion happens after tilde expansion, so MySQL was handed a path it could not resolve - the Cacti checkout took the default branch, which is 1.3 in development and whose CLI installer currently fatals with an undefined __() plugin_syslog additionally installed libapache2-mod-php${{ matrix.php }}, which Ubuntu does not package, so apt exited 100 before Cacti was reached. Verified with actionlint, which is clean on the result. Signed-off-by: Thomas Vincent --- .github/workflows/plugin-ci-workflow.yml | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 3840fa29..d22ab2d8 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -40,6 +40,7 @@ jobs: uses: actions/checkout@v4 with: repository: Cacti/cacti + ref: release/1.2.31 path: cacti - name: Checkout Syslog Plugin @@ -57,7 +58,7 @@ jobs: - name: Check PHP Syntax (Lint) run: | cd cacti/plugins/syslog - if find . -name '*.php' -not -path './vendor/*' -exec php -l {} 2>&1 \; | grep -iv 'no syntax errors detected'; then + if find . -name '*.php' -not -path './vendor/*' -exec php -l {} \; 2>&1 | grep -iv 'no syntax errors detected'; then echo "Syntax errors found!" exit 1 fi @@ -122,6 +123,7 @@ jobs: uses: actions/checkout@v4 with: repository: Cacti/cacti + ref: release/1.2.31 path: cacti - name: Checkout Syslog Plugin @@ -163,16 +165,15 @@ jobs: echo -e "[client]\nuser = root\npassword = cactiroot\nhost = 127.0.0.1\n" > ~/.my.cnf - name: Initialize Cacti Database - env: - MYSQL_AUTH_USR: '--defaults-file=~/.my.cnf' run: | - mysql $MYSQL_AUTH_USR -e 'CREATE DATABASE IF NOT EXISTS cacti;' - mysql $MYSQL_AUTH_USR -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';" - mysql $MYSQL_AUTH_USR -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';" - mysql $MYSQL_AUTH_USR -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';" - mysql $MYSQL_AUTH_USR -e "FLUSH PRIVILEGES;" - mysql $MYSQL_AUTH_USR cacti < ${{ github.workspace }}/cacti/cacti.sql - mysql $MYSQL_AUTH_USR -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti + MYSQL_AUTH_USR="--defaults-file=$HOME/.my.cnf" + mysql "$MYSQL_AUTH_USR" -e 'CREATE DATABASE IF NOT EXISTS cacti;' + mysql "$MYSQL_AUTH_USR" -e "CREATE USER IF NOT EXISTS 'cactiuser'@'localhost' IDENTIFIED BY 'cactiuser';" + mysql "$MYSQL_AUTH_USR" -e "GRANT ALL PRIVILEGES ON cacti.* TO 'cactiuser'@'localhost';" + mysql "$MYSQL_AUTH_USR" -e "GRANT SELECT ON mysql.time_zone_name TO 'cactiuser'@'localhost';" + mysql "$MYSQL_AUTH_USR" -e "FLUSH PRIVILEGES;" + mysql "$MYSQL_AUTH_USR" cacti < ${{ github.workspace }}/cacti/cacti.sql + mysql "$MYSQL_AUTH_USR" -e "INSERT INTO settings (name, value) VALUES ('path_php_binary', '/usr/bin/php')" cacti - name: Validate composer files run: | From 3c6ccc588782a8d3228f9690ef7959cbed117119 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 3 Sep 2026 13:26:40 -0700 Subject: [PATCH 14/19] refactor: rename the JSON script encoder to syslog_json_safe Drops the typed signature to match the rest of functions.php, where no function declares parameter or return types. Signed-off-by: Thomas Vincent --- functions.php | 6 +++++- setup.php | 6 +++--- syslog.php | 10 +++++----- tests/regression/issue259_csrf_purge_test.php | 10 +++++----- .../issue279_bulk_form_and_nav_encoding_test.php | 10 +++++----- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/functions.php b/functions.php index d39c4352..73fae2c6 100644 --- a/functions.php +++ b/functions.php @@ -47,7 +47,11 @@ function syslog_include_js() { block, because the browser does + * not HTML-decode there. The value has to arrive as a JSON literal. + */ +function syslog_json_safe($value) { return json_encode($value, JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_THROW_ON_ERROR); } diff --git a/setup.php b/setup.php index 8461562b..eec88af7 100644 --- a/setup.php +++ b/setup.php @@ -1692,21 +1692,21 @@ function syslog_utilities_list() { $(function() { $('#syslog_purge_hosts').on('click', function() { $('#syslog_purge_dialog').dialog({ - title: , + title: , minHeight: 80, minWidth: 400, resizable: false, draggable: true, buttons: { 'Cancel': { - text: , + text: , id: 'btnPurgeCancel', click: function() { $(this).dialog('close'); } }, 'Continue': { - text: , + text: , id: 'btnPurgeContinue', click: function() { $(this).dialog('close'); diff --git a/syslog.php b/syslog.php index 80165d8f..4cb40dee 100644 --- a/syslog.php +++ b/syslog.php @@ -1184,11 +1184,11 @@ function syslog_filter($sql_where, $tab) { ?> ,")) { +if (!str_contains($syslog, "pageTab: ,")) { fwrite(STDERR, "Expected JSON-encoded syslog pageTab value\n"); exit(1); } foreach ([ - "syslog_json_encode_for_script(__('Enter a search term', 'syslog'))", - "syslog_json_encode_for_script(__('Select Device(s)', 'syslog'))", - "syslog_json_encode_for_script(__('Devices Selected', 'syslog'))", - "syslog_json_encode_for_script(__('All Devices Selected', 'syslog'))", + "syslog_json_safe(__('Enter a search term', 'syslog'))", + "syslog_json_safe(__('Select Device(s)', 'syslog'))", + "syslog_json_safe(__('Devices Selected', 'syslog'))", + "syslog_json_safe(__('All Devices Selected', 'syslog'))", ] as $needle) { if (!str_contains($syslog, $needle)) { fwrite(STDERR, "Expected JS-safe initSyslogMain text encoding\n"); From fb4b20a8ed88991794dd949b8bc90299b596bbf0 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 3 Sep 2026 17:15:15 -0700 Subject: [PATCH 15/19] fix: keep the purge post headerless The install form in this file already appends header=false, and the redirect targets need it too or the XHR renders a full page into the content tab. Signed-off-by: Thomas Vincent --- setup.php | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/setup.php b/setup.php index eec88af7..6aa67fd0 100644 --- a/setup.php +++ b/setup.php @@ -1611,7 +1611,7 @@ function syslog_utilities_action($action) { if ($_SERVER['REQUEST_METHOD'] !== 'POST') { cacti_log('WARNING: syslog purge blocked -- non-POST request', false, 'SYSLOG'); raise_message('syslog_method_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR); - header('Location: utilities.php'); + header('Location: utilities.php?header=false'); exit; } @@ -1621,14 +1621,14 @@ function syslog_utilities_action($action) { if (!function_exists('csrf_check')) { cacti_log('WARNING: syslog purge blocked -- CSRF validation unavailable', false, 'SYSLOG'); raise_message('syslog_csrf_unavailable', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR); - header('Location: utilities.php'); + header('Location: utilities.php?header=false'); exit; } if (!csrf_check(false)) { cacti_log('WARNING: syslog purge blocked -- CSRF token validation failed', false, 'SYSLOG'); raise_message('syslog_csrf_error', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR); - header('Location: utilities.php'); + header('Location: utilities.php?header=false'); exit; } @@ -1666,7 +1666,7 @@ function syslog_utilities_action($action) { raise_message('syslog_info', __('There were %s Device records removed from the Syslog database', $records, 'syslog'), MESSAGE_LEVEL_INFO); - header('Location: utilities.php'); + header('Location: utilities.php?header=false'); exit; } @@ -1711,15 +1711,17 @@ function syslog_utilities_list() { click: function() { $(this).dialog('close'); - const postData = { - action: 'purge_syslog_hosts', - __csrf_magic: csrfMagicToken - }; + /* set the URL */ + var strURL = 'utilities.php?header=false'; - if (typeof postUrl === 'function') { - postUrl({url: 'utilities.php', noState: true}, postData); + /* ensure that the csrf magic is appended */ + var json = {action: 'purge_syslog_hosts'}; + json.__csrf_magic = csrfMagicToken; + + if (typeof postUrl == 'function') { + postUrl({url: strURL}, json); } else { - loadPageUsingPost('utilities.php', postData); + loadPageUsingPost(strURL, json); } } } From 3ae858dab9f05c4aec9d5e4819a0f1b915f4355d Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 3 Sep 2026 17:16:20 -0700 Subject: [PATCH 16/19] refactor: drop the nav bar filter encoding from this branch The issue#279 XSS fix is unrelated to the purge CSRF guard and moves to its own branch. Signed-off-by: Thomas Vincent --- syslog_alerts.php | 4 +- syslog_removal.php | 4 +- syslog_reports.php | 4 +- ...sue279_bulk_form_and_nav_encoding_test.php | 58 ------------------- 4 files changed, 6 insertions(+), 64 deletions(-) delete mode 100644 tests/regression/issue279_bulk_form_and_nav_encoding_test.php diff --git a/syslog_alerts.php b/syslog_alerts.php index b97f8c93..6f47c97c 100644 --- a/syslog_alerts.php +++ b/syslog_alerts.php @@ -210,7 +210,7 @@ function form_actions() { - + $save_html "; @@ -856,7 +856,7 @@ function syslog_alerts() { 'user' => [__('By User', 'syslog'), 'DESC'] ]; - $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'); + $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'); form_start('syslog_alerts.php', 'chk'); diff --git a/syslog_removal.php b/syslog_removal.php index b8768b5a..e047e68c 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=' . rawurlencode(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=' . 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 ecf2ef16..fd7b2120 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=' . rawurlencode(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=' . 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 deleted file mode 100644 index 6c573b6e..00000000 --- a/tests/regression/issue279_bulk_form_and_nav_encoding_test.php +++ /dev/null @@ -1,58 +0,0 @@ - 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 (['syslog_removal.php', 'syslog_alerts.php', 'syslog_reports.php'] as $file) { - if (!str_contains($targets[$file], "html_escape(get_request_var('drp_action'))")) { - fwrite(STDERR, "Expected escaped drp_action hidden field in $file\n"); - exit(1); - } - - if (!str_contains($targets[$file], "rawurlencode(get_request_var('filter'))")) { - fwrite(STDERR, "Expected URL-encoded filter nav value in $file\n"); - exit(1); - } - - if (str_contains($targets[$file], "")) { - fwrite(STDERR, "Legacy raw drp_action hidden field remains in $file\n"); - exit(1); - } -} - -$syslog = $targets['syslog.php']; - -if (!str_contains($syslog, "pageTab: ,")) { - fwrite(STDERR, "Expected JSON-encoded syslog pageTab value\n"); - exit(1); -} - -foreach ([ - "syslog_json_safe(__('Enter a search term', 'syslog'))", - "syslog_json_safe(__('Select Device(s)', 'syslog'))", - "syslog_json_safe(__('Devices Selected', 'syslog'))", - "syslog_json_safe(__('All Devices Selected', 'syslog'))", -] as $needle) { - if (!str_contains($syslog, $needle)) { - fwrite(STDERR, "Expected JS-safe initSyslogMain text encoding\n"); - exit(1); - } -} - -if (str_contains($syslog, "pageTab: ''")) { - fwrite(STDERR, "Legacy raw pageTab JS assignment still present\n"); - exit(1); -} - -echo "OK\n"; From 863c3fed286cce44032918e07446dcf4d5a2a810 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 3 Sep 2026 17:19:06 -0700 Subject: [PATCH 17/19] test: drive the purge guard instead of grepping for it Every assertion passed while the purge was broken, and two of them rejected correct fixes; the guard and the fragment are now exercised for real. Signed-off-by: Thomas Vincent --- tests/regression/issue259_csrf_purge_test.php | 310 +++++++++++++----- 1 file changed, 226 insertions(+), 84 deletions(-) diff --git a/tests/regression/issue259_csrf_purge_test.php b/tests/regression/issue259_csrf_purge_test.php index c0637725..5b102b80 100644 --- a/tests/regression/issue259_csrf_purge_test.php +++ b/tests/regression/issue259_csrf_purge_test.php @@ -1,127 +1,269 @@ must not close the block it sits in. + */ +$payload = '\'"&'; + +$harness = <<<'HARNESS' +&1'; + + return shell_exec($command); +} + +/* the encoder, on its own */ + +require_once $root . '/functions.php'; + +$encoded = syslog_json_safe($payload); + +if (json_decode($encoded) !== $payload) { + issue259_fail('syslog_json_safe() does not round-trip through json_decode()'); +} + +foreach (['<', '>', '&', '"', '\''] as $raw) { + if (strpos(substr($encoded, 1, -1), $raw) !== false) { + issue259_fail("syslog_json_safe() left a raw $raw in the JS literal"); + } +} + +/* the utilities fragment, rendered */ + +$render = issue259_run($sandbox, 'render', $payload); + +if (strpos($render, 'REACHED_END') === false) { + issue259_fail("syslog_utilities_list() did not complete:\n$render"); } -// Verify fail-closed: the else branch (csrf_check unavailable) must reject, not fall through. -// Assert globally safe properties rather than parsing the else block via brittle regex. +if (stripos($render, '') !== 1) { + issue259_fail('A translated string escaped its script block'); } -// The fallback path must log the blocked attempt -if (!str_contains($setup, "cacti_log('WARNING: syslog purge blocked")) { - fwrite(STDERR, "Fail-closed branch must call cacti_log() to audit blocked purge attempts.\n"); - exit(1); +if (!preg_match('/title:\s*("[^"]*"),/', $render, $title)) { + issue259_fail('The dialog title is not a JSON literal'); } -// Log message must name the specific failure reason for incident response -if (!str_contains($setup, 'CSRF validation unavailable')) { - fwrite(STDERR, "Log message must specify 'CSRF validation unavailable' for operational clarity.\n"); - exit(1); +if (json_decode($title[1]) !== $payload) { + issue259_fail('The dialog title does not decode back to the translated text'); } -// Verify JS confirm() uses json_encode, not __esc() inside JS string -if (preg_match("/confirm\(\s*'/", $setup)) { - fwrite(STDERR, "JS confirm() must use json_encode() for safe encoding, not __esc() in a quoted string.\n"); - exit(1); +if (strpos($render, "'utilities.php?header=false'") === false) { + issue259_fail('The purge post must target the headerless utilities page'); } -if (!str_contains($setup, "syslog_json_safe(__('Confirm Purge', 'syslog'))")) { - fwrite(STDERR, "Expected syslog_json_safe() for JS-safe dialog title encoding.\n"); - exit(1); +if (strpos($render, 'json.__csrf_magic = csrfMagicToken;') === false) { + issue259_fail('The purge post must carry the CSRF token'); } -if (!str_contains($setup, "syslog_json_safe(__('Cancel', 'syslog'))") || - !str_contains($setup, "syslog_json_safe(__('Continue', 'syslog'))")) { - fwrite(STDERR, "Expected syslog_json_safe() for JS-safe button text encoding.\n"); - exit(1); +/* the guard, driven */ + +$blocked = [ + 'action_get' => 'non-POST request', + 'action_no_csrf' => 'CSRF validation unavailable', + 'action_bad_token' => 'CSRF token validation failed' +]; + +foreach ($blocked as $scenario => $reason) { + $output = issue259_run($sandbox, $scenario, $payload); + + if (strpos($output, 'DBEXEC') !== false) { + issue259_fail("$scenario reached the purge deletes"); + } + + if (strpos($output, "LOG:WARNING: syslog purge blocked -- $reason") === false) { + issue259_fail("$scenario did not audit the block as '$reason':\n$output"); + } + + if (!preg_match('/^MSG:(\S+)\|([^|]*)\|(\d+)$/m', $output, $message)) { + issue259_fail("$scenario did not raise a user-visible message:\n$output"); + } + + if ($message[3] != MESSAGE_LEVEL_ERROR) { + issue259_fail("$scenario raised the block at level $message[3], not error"); + } + + if ($message[2] !== 'Invalid request. Please try again.') { + issue259_fail("$scenario used the wrong user-facing text: $message[2]"); + } + + if (stripos($message[2], 'csrf') !== false) { + issue259_fail("$scenario leaked CSRF internals to the user"); + } } -// Verify json_encode uses JSON_HEX_TAG to prevent breakout in HTML script context -if (!str_contains($functions, 'JSON_HEX_TAG')) { - fwrite(STDERR, "json_encode() must use JSON_HEX_TAG to prevent script-context breakout.\n"); - exit(1); +$allowed = issue259_run($sandbox, 'action_valid_token', $payload); + +if (substr_count($allowed, 'DBEXEC') !== 3) { + issue259_fail("A POST with a valid token must run all three deletes:\n$allowed"); } -if (!str_contains($functions, 'JSON_HEX_AMP')) { - fwrite(STDERR, "json_encode() must use JSON_HEX_AMP to escape ampersands in script context.\n"); - exit(1); +if (strpos($allowed, 'MSG:syslog_info|') === false) { + issue259_fail("A completed purge must report the record count:\n$allowed"); } -if (!str_contains($functions, 'JSON_HEX_APOS')) { - fwrite(STDERR, "json_encode() must use JSON_HEX_APOS.\n"); - exit(1); +/* source scan for what the CLI SAPI cannot observe */ + +$setup = file_get_contents($root . '/setup.php'); + +if ($setup === false) { + issue259_fail('Failed to load setup.php'); } -if (!str_contains($functions, 'JSON_HEX_QUOT')) { - fwrite(STDERR, "json_encode() must use JSON_HEX_QUOT.\n"); - exit(1); +if (str_contains($setup, "href='utilities.php?action=purge_syslog_hosts'")) { + issue259_fail('The GET purge link is still present'); } -// Verify user-facing messages do not expose CSRF internals (log messages may use "CSRF") -if (preg_match('/raise_message\\s*\\(\\s*[^,]+,\\s*__\\(\\s*([\'\"])[^\'\"]*CSRF[^\'\"]*\\1/si', $setup)) { - fwrite(STDERR, "User-facing raise_message must not expose CSRF internals to end users.\n"); - exit(1); +if (preg_match("/header\('Location: utilities\.php'\)/", $setup)) { + issue259_fail('A purge redirect target is missing header=false'); } -// Verify generic user-facing message is present -if (!str_contains($setup, 'Invalid request. Please try again.')) { - fwrite(STDERR, "Fail-closed branch must use generic 'Invalid request. Please try again.' message.\n"); - exit(1); +if (substr_count($setup, "header('Location: utilities.php?header=false');") !== 4) { + issue259_fail('Every purge exit path must redirect to the headerless page'); } -// Verify fail-closed raise_message uses MESSAGE_LEVEL_ERROR severity -if (!str_contains($setup, "raise_message('syslog_csrf_unavailable', __('Invalid request. Please try again.', 'syslog'), MESSAGE_LEVEL_ERROR)")) { - fwrite(STDERR, "Fail-closed branch raise_message must use MESSAGE_LEVEL_ERROR severity.\n"); - exit(1); +$syslog = file_get_contents($root . '/syslog.php'); + +if ($syslog === false) { + issue259_fail('Failed to load syslog.php'); } -// Verify log message does not expose internal function name -if (str_contains($setup, 'csrf_check() unavailable')) { - fwrite(STDERR, "Log message must not name internal validation function.\n"); - exit(1); +foreach ([ + "pageTab: ,", + "syslog_json_safe(__('Enter a search term', 'syslog'))", + "syslog_json_safe(__('Select Device(s)', 'syslog'))", + "syslog_json_safe(__('Devices Selected', 'syslog'))", + "syslog_json_safe(__('All Devices Selected', 'syslog'))" +] as $snippet) { + if (!str_contains($syslog, $snippet)) { + issue259_fail("initSyslogMain is not JS-encoded: $snippet"); + } } -echo "issue259_csrf_purge_test passed\n"; +print "$name passed\n"; From 5b44732a707536abcbc603fbede1e51d2045a294 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 3 Sep 2026 17:20:11 -0700 Subject: [PATCH 18/19] docs: record issue#259 in the changelog Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a22b427..6be2679b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ * issue#252: hardening: escape device hostname output in syslog view; parameterize alert API functions * issue#256: hardening: prevent CSV formula injection and malformed CSV output in exports * issue#258: Execute CREATE TABLE SQL correctly during replication sync +* issue#259: hardening: require POST and a CSRF token for the purge syslog devices utility * issue#260: hardening: replace eval-based callback execution in syslog autocomplete JS * issue#278: Extract duplicated alert command execution paths in syslog_process_alerts * issue#278: Extract alert command execution into shared helper in functions.php; command tokenization now uses preg_split (handles tabs and consecutive spaces); /bin/sh fallback for non-executable command templates removed (use absolute paths with execute bit set) From ccf0c99e6390f5fdc53e4b9e30a2bfad86a24d6d Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 3 Sep 2026 17:23:55 -0700 Subject: [PATCH 19/19] style: space the closing php tags like the rest of the repo Every other close tag in the tree is '; ?>'; these five were the only exceptions. Signed-off-by: Thomas Vincent --- functions.php | 4 ++++ setup.php | 10 +++++----- syslog.php | 10 +++++----- tests/regression/issue259_csrf_purge_test.php | 2 +- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/functions.php b/functions.php index 73fae2c6..032ae97c 100644 --- a/functions.php +++ b/functions.php @@ -50,6 +50,10 @@ function syslog_include_js() { /** * __esc() is not enough inside a ,", + "pageTab: ,", "syslog_json_safe(__('Enter a search term', 'syslog'))", "syslog_json_safe(__('Select Device(s)', 'syslog'))", "syslog_json_safe(__('Devices Selected', 'syslog'))",