From bbc7cc03f269a9d6ea1122ce3aa4c919e89673a6 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 6 Mar 2026 18:55:51 -0800 Subject: [PATCH 1/3] refactor: deduplicate selected-items bulk action dispatch Fixes #276 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + functions.php | 14 ++++++- syslog_alerts.php | 32 +++++++-------- syslog_removal.php | 39 +++++++------------ syslog_reports.php | 34 +++++++--------- ...ue276_bulk_action_dispatch_helper_test.php | 36 +++++++++++++++++ 6 files changed, 92 insertions(+), 64 deletions(-) create mode 100644 tests/regression/issue276_bulk_action_dispatch_helper_test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1e7f2..414bdc9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* issue#276: Deduplicate selected-item bulk action dispatch across alert/report/removal pages * 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/functions.php b/functions.php index 9bd3223..957d64f 100644 --- a/functions.php +++ b/functions.php @@ -126,6 +126,19 @@ function syslog_sendemail($to, $from, $subject, $message, $smsmessage = '') { } } +function syslog_apply_selected_items_action($selected_items, $drp_action, $action_map, $export_action = '', $export_items = '') { + if ($selected_items != false) { + if (isset($action_map[$drp_action])) { + foreach($selected_items as $selected_item) { + $action_function = $action_map[$drp_action]; + $action_function($selected_item); + } + } elseif ($export_action != '' && $drp_action == $export_action) { + $_SESSION['exporter'] = $export_items; + } + } +} + function syslog_is_partitioned() { global $syslogdb_default; @@ -2421,4 +2434,3 @@ function alert_replace_variables($alert, $results, $hostname = '') { return $command; } - diff --git a/syslog_alerts.php b/syslog_alerts.php index 74c49b9..5ddc44c 100644 --- a/syslog_alerts.php +++ b/syslog_alerts.php @@ -116,24 +116,19 @@ function form_actions() { /* if we are to save this form, instead of display it */ if (isset_request_var('selected_items')) { $selected_items = sanitize_unserialize_selected_items(get_request_var('selected_items')); - - if ($selected_items != false) { - if (get_request_var('drp_action') == '1') { /* delete */ - for ($i=0; $i 'api_syslog_alert_remove', + '2' => 'api_syslog_alert_disable', + '3' => 'api_syslog_alert_enable' + ), + '4', + get_nfilter_request_var('selected_items') + ); header('Location: syslog_alerts.php?header=false'); @@ -1009,4 +1004,3 @@ function alert_import() { header('Location: syslog_alerts.php'); } - diff --git a/syslog_removal.php b/syslog_removal.php index f7b5e94..12e308b 100644 --- a/syslog_removal.php +++ b/syslog_removal.php @@ -120,29 +120,21 @@ function form_actions() { /* if we are to save this form, instead of display it */ if (isset_request_var('selected_items')) { - $selected_items = sanitize_unserialize_selected_items(get_nfilter_request_var('selected_items')); - - if ($selected_items != false) { - if (get_request_var('drp_action') == '1') { /* delete */ - for ($i=0; $i 'api_syslog_removal_remove', + '2' => 'api_syslog_removal_disable', + '3' => 'api_syslog_removal_enable', + '4' => 'api_syslog_removal_reprocess' + ), + '5', + get_nfilter_request_var('selected_items') + ); header('Location: syslog_removal.php?header=false'); @@ -810,4 +802,3 @@ function removal_import() { header('Location: syslog_removal.php'); } - diff --git a/syslog_reports.php b/syslog_reports.php index f0caec2..a9d05d8 100644 --- a/syslog_reports.php +++ b/syslog_reports.php @@ -112,25 +112,20 @@ function form_actions() { /* if we are to save this form, instead of display it */ if (isset_request_var('selected_items')) { - $selected_items = sanitize_unserialize_selected_items(get_nfilter_request_var('selected_items')); - - if ($selected_items != false) { - if (get_request_var('drp_action') == '1') { /* delete */ - for ($i=0; $i 'api_syslog_report_remove', + '2' => 'api_syslog_report_disable', + '3' => 'api_syslog_report_enable' + ), + '4', + get_nfilter_request_var('selected_items') + ); header('Location: syslog_reports.php?header=false'); @@ -872,4 +867,3 @@ function report_import() { header('Location: syslog_reports.php'); } - diff --git a/tests/regression/issue276_bulk_action_dispatch_helper_test.php b/tests/regression/issue276_bulk_action_dispatch_helper_test.php new file mode 100644 index 0000000..816f7db --- /dev/null +++ b/tests/regression/issue276_bulk_action_dispatch_helper_test.php @@ -0,0 +1,36 @@ + Date: Fri, 6 Mar 2026 22:58:16 -0800 Subject: [PATCH 2/3] fix: encode session exporter value to prevent JS injection rawurlencode(serialize($selected_items)) replaces storing the raw request string directly in $_SESSION['exporter']. The raw value was later embedded verbatim in a document.location JS string, which allowed XSS if the serialized payload contained quote or bracket characters. Use the already-sanitized $selected_items array, re-serialize it, then URL-encode before session storage so the JS embed is always safe. Also tighten the regression test to use preg_match instead of strpos so whitespace variations in the function signature do not cause false failures. Signed-off-by: Thomas Vincent --- functions.php | 5 ++++- .../regression/issue276_bulk_action_dispatch_helper_test.php | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/functions.php b/functions.php index 957d64f..6338cb6 100644 --- a/functions.php +++ b/functions.php @@ -134,7 +134,10 @@ function syslog_apply_selected_items_action($selected_items, $drp_action, $actio $action_function($selected_item); } } elseif ($export_action != '' && $drp_action == $export_action) { - $_SESSION['exporter'] = $export_items; + /* Re-serialize the sanitized array and URL-encode so the value is + * safe to embed in a JS document.location string (avoids injection + * via the raw request value that $export_items carries). */ + $_SESSION['exporter'] = rawurlencode(serialize($selected_items)); } } } diff --git a/tests/regression/issue276_bulk_action_dispatch_helper_test.php b/tests/regression/issue276_bulk_action_dispatch_helper_test.php index 816f7db..cec8251 100644 --- a/tests/regression/issue276_bulk_action_dispatch_helper_test.php +++ b/tests/regression/issue276_bulk_action_dispatch_helper_test.php @@ -8,7 +8,7 @@ exit(1); } -if (strpos($functions, 'function syslog_apply_selected_items_action(') === false) { +if (!preg_match('/function\s+syslog_apply_selected_items_action\s*\(/', $functions)) { fwrite(STDERR, "Shared selected-items action helper is missing.\n"); exit(1); } @@ -27,7 +27,7 @@ exit(1); } - if (strpos($content, 'syslog_apply_selected_items_action(') === false) { + if (!preg_match('/syslog_apply_selected_items_action\s*\(/', $content)) { fwrite(STDERR, "Shared selected-items action helper is not used in $target\n"); exit(1); } From a954f85ef273e53876bcec92bfde40ebd0bd86e7 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 7 Mar 2026 17:36:10 -0800 Subject: [PATCH 3/3] hardening: guard variable function call with function_exists() check Prevent a fatal error if an action map entry names a non-existent function. Also fix indentation in the export elseif block. Signed-off-by: Thomas Vincent --- functions.php | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/functions.php b/functions.php index 6338cb6..e434bd1 100644 --- a/functions.php +++ b/functions.php @@ -129,15 +129,20 @@ function syslog_sendemail($to, $from, $subject, $message, $smsmessage = '') { function syslog_apply_selected_items_action($selected_items, $drp_action, $action_map, $export_action = '', $export_items = '') { if ($selected_items != false) { if (isset($action_map[$drp_action])) { - foreach($selected_items as $selected_item) { - $action_function = $action_map[$drp_action]; - $action_function($selected_item); + $action_function = $action_map[$drp_action]; + + if (function_exists($action_function)) { + foreach($selected_items as $selected_item) { + $action_function($selected_item); + } + } else { + cacti_log("SYSLOG ERROR: Bulk action function '$action_function' not found.", false, 'SYSTEM'); } } elseif ($export_action != '' && $drp_action == $export_action) { /* Re-serialize the sanitized array and URL-encode so the value is - * safe to embed in a JS document.location string (avoids injection - * via the raw request value that $export_items carries). */ - $_SESSION['exporter'] = rawurlencode(serialize($selected_items)); + * safe to embed in a JS document.location string (avoids injection + * via the raw request value that $export_items carries). */ + $_SESSION['exporter'] = rawurlencode(serialize($selected_items)); } } }