From 5325cc829f3a030f1dc78c0337eda407b7325209 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 6 Mar 2026 14:03:32 -0800 Subject: [PATCH 1/7] hardening: require POST+CSRF for syslog purge Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent --- setup.php | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) diff --git a/setup.php b/setup.php index a90cb01..c85543c 100644 --- a/setup.php +++ b/setup.php @@ -1566,6 +1566,12 @@ function syslog_utilities_action($action) { } if ($action == 'purge_syslog_hosts') { + if ($_SERVER['REQUEST_METHOD'] !== 'POST' || !isset($_POST['__csrf_magic'])) { + raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); + header('Location: utilities.php'); + exit; + } + $records = 0; syslog_db_execute('DELETE FROM syslog_hosts @@ -1618,7 +1624,27 @@ function syslog_utilities_list() { - + '> + @@ -1626,4 +1652,3 @@ function syslog_utilities_list() { Date: Fri, 6 Mar 2026 14:42:54 -0800 Subject: [PATCH 2/7] hardening: validate CSRF token for purge utility action Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent --- .github/workflows/plugin-ci-workflow.yml | 10 +++++++ CHANGELOG.md | 1 + setup.php | 14 ++++++++- tests/regression/issue259_csrf_purge_test.php | 29 +++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tests/regression/issue259_csrf_purge_test.php diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 7580ee1..76612ac 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -187,6 +187,16 @@ jobs: echo "Syntax errors found!" exit 1 fi + + - name: Run Plugin Regression Tests + run: | + cd ${{ github.workspace }}/cacti/plugins/syslog + if [ -d tests/regression ]; then + for test in tests/regression/*.php; do + [ -f "$test" ] || continue + php "$test" + done + fi - name: Run Cacti Poller diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1e7f2..f1ee630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* issue#259: Require POST + CSRF token for purge syslog devices utility action * issue: Making changes to support Cacti 1.3 * issue: Don't use MyISAM for non-analytical tables * issue: The install advisor for Syslog was broken in current Cacti releases diff --git a/setup.php b/setup.php index c85543c..9379c6d 100644 --- a/setup.php +++ b/setup.php @@ -1566,7 +1566,19 @@ function syslog_utilities_action($action) { } if ($action == 'purge_syslog_hosts') { - if ($_SERVER['REQUEST_METHOD'] !== 'POST' || !isset($_POST['__csrf_magic'])) { + 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; + } + } elseif (!isset($_POST['__csrf_magic']) || trim($_POST['__csrf_magic']) === '') { raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', '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 new file mode 100644 index 0000000..0969bb9 --- /dev/null +++ b/tests/regression/issue259_csrf_purge_test.php @@ -0,0 +1,29 @@ + Date: Sun, 8 Mar 2026 03:57:50 -0700 Subject: [PATCH 3/7] chore: add CHANGELOG entry for develop Signed-off-by: Thomas Vincent --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f1ee630..0ab10e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ --- develop --- -* issue#259: Require POST + CSRF token for purge syslog devices utility action +* issue#259: Require POST and validate CSRF token before executing purge_syslog_hosts * issue: Making changes to support Cacti 1.3 * issue: Don't use MyISAM for non-analytical tables * issue: The install advisor for Syslog was broken in current Cacti releases From 6e76ca2a1c8a21ef17888134116840ebf6ec6272 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 8 Mar 2026 20:19:55 -0700 Subject: [PATCH 4/7] hardening: fail-closed CSRF fallback and fix JS encoding in purge utility Signed-off-by: Thomas Vincent --- setup.php | 6 +++--- tests/regression/issue259_csrf_purge_test.php | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/setup.php b/setup.php index 9379c6d..80b0c2a 100644 --- a/setup.php +++ b/setup.php @@ -1578,8 +1578,8 @@ function syslog_utilities_action($action) { header('Location: utilities.php'); exit; } - } elseif (!isset($_POST['__csrf_magic']) || trim($_POST['__csrf_magic']) === '') { - raise_message('syslog_error', __('Invalid request. This action requires a CSRF protected POST.', 'syslog'), MESSAGE_LEVEL_ERROR); + } else { + raise_message('syslog_error', __('CSRF validation unavailable. Action denied.', 'syslog'), MESSAGE_LEVEL_ERROR); header('Location: utilities.php'); exit; } @@ -1640,7 +1640,7 @@ function syslog_utilities_list() { 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 error message does not expose internal CSRF subsystem state +if (strpos($setup, 'CSRF validation unavailable') !== false) { + fwrite(STDERR, "Error message must not reveal CSRF subsystem state to users.\n"); + exit(1); +} + echo "issue259_csrf_purge_test passed\n"; From d39a83bfef4fe2277489684e349b00078f442f56 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 8 Mar 2026 20:26:45 -0700 Subject: [PATCH 6/7] hardening: remove internal function name from log, add positive test assertions Signed-off-by: Thomas Vincent --- setup.php | 2 +- tests/regression/issue259_csrf_purge_test.php | 28 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/setup.php b/setup.php index f9359dd..a3d40ac 100644 --- a/setup.php +++ b/setup.php @@ -1579,7 +1579,7 @@ function syslog_utilities_action($action) { exit; } } else { - cacti_log('WARNING: syslog purge blocked -- csrf_check() unavailable', false, 'SYSLOG'); + cacti_log('WARNING: syslog purge blocked -- security validation unavailable', false, 'SYSLOG'); raise_message('syslog_error', __('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 cdaf682..c25ae75 100644 --- a/tests/regression/issue259_csrf_purge_test.php +++ b/tests/regression/issue259_csrf_purge_test.php @@ -30,11 +30,17 @@ // Use two separate assertions instead of a single fragile regex: // 1) The else branch must exist (fail-closed path) // 2) $_POST['__csrf_magic'] must not appear anywhere in the else/fallback path -if (preg_match('/\}\s*else\s*\{([\s\S]*?)\n\t\}/m', $setup, $elseMatch)) { - if (strpos($elseMatch[1], "\$_POST['__csrf_magic']") !== false) { - fwrite(STDERR, "Fallback CSRF branch must not check token presence; must fail closed.\n"); - exit(1); - } +if (!preg_match('/\}\s*else\s*\{([\s\S]*?)\n\t\}/m', $setup, $elseMatch)) { + fwrite(STDERR, "Could not locate else branch in setup.php; test is stale.\n"); + exit(1); +} +if (strpos($elseMatch[1], "\$_POST['__csrf_magic']") !== false) { + fwrite(STDERR, "Fallback CSRF branch must not check token presence; must fail closed.\n"); + exit(1); +} +if (strpos($elseMatch[1], "cacti_log(") === false) { + fwrite(STDERR, "Fail-closed branch must call cacti_log() to audit blocked purge attempts.\n"); + exit(1); } // Verify JS confirm() uses json_encode, not __esc() inside JS string @@ -75,4 +81,16 @@ 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 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 3dc734f0b4f160dfa1b1cde27d29948ee098752c Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 8 Mar 2026 20:29:53 -0700 Subject: [PATCH 7/7] hardening: use CSRF wording in log, simplify test assertions to strpos checks Signed-off-by: Thomas Vincent --- setup.php | 2 +- tests/regression/issue259_csrf_purge_test.php | 36 ++++++++++++------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/setup.php b/setup.php index a3d40ac..298c2f5 100644 --- a/setup.php +++ b/setup.php @@ -1579,7 +1579,7 @@ function syslog_utilities_action($action) { exit; } } else { - cacti_log('WARNING: syslog purge blocked -- security validation unavailable', false, 'SYSLOG'); + 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; diff --git a/tests/regression/issue259_csrf_purge_test.php b/tests/regression/issue259_csrf_purge_test.php index c25ae75..54bdac2 100644 --- a/tests/regression/issue259_csrf_purge_test.php +++ b/tests/regression/issue259_csrf_purge_test.php @@ -26,23 +26,27 @@ exit(1); } -// Verify fail-closed: when csrf_check is unavailable, request is rejected (no fallback). -// Use two separate assertions instead of a single fragile regex: -// 1) The else branch must exist (fail-closed path) -// 2) $_POST['__csrf_magic'] must not appear anywhere in the else/fallback path -if (!preg_match('/\}\s*else\s*\{([\s\S]*?)\n\t\}/m', $setup, $elseMatch)) { - fwrite(STDERR, "Could not locate else branch in setup.php; test is stale.\n"); - exit(1); -} -if (strpos($elseMatch[1], "\$_POST['__csrf_magic']") !== false) { +// 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. + +// The fallback path must not attempt manual token checking +if (strpos($setup, "\$_POST['__csrf_magic']") !== false) { fwrite(STDERR, "Fallback CSRF branch must not check token presence; must fail closed.\n"); exit(1); } -if (strpos($elseMatch[1], "cacti_log(") === false) { + +// The fallback path must log the blocked attempt +if (strpos($setup, "cacti_log('WARNING: syslog purge blocked") === false) { fwrite(STDERR, "Fail-closed branch must call cacti_log() to audit blocked purge attempts.\n"); exit(1); } +// Log message must name the specific failure reason for incident response +if (strpos($setup, 'CSRF validation unavailable') === false) { + fwrite(STDERR, "Log message must specify 'CSRF validation unavailable' for operational clarity.\n"); + exit(1); +} + // 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"); @@ -75,9 +79,9 @@ exit(1); } -// Verify error message does not expose internal CSRF subsystem state -if (strpos($setup, 'CSRF validation unavailable') !== false) { - fwrite(STDERR, "Error message must not reveal CSRF subsystem state to users.\n"); +// 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); } @@ -87,6 +91,12 @@ 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");