From c1b64287bbd69e167c7484f615e1870520c4abe2 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Fri, 6 Mar 2026 18:35:57 -0800 Subject: [PATCH 1/4] hardening: remove shell fallback from alert command execution Fixes #257 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Thomas Vincent --- CHANGELOG.md | 1 + functions.php | 19 +++++++----- .../issue257_remove_shell_fallback_test.php | 30 +++++++++++++++++++ 3 files changed, 43 insertions(+), 7 deletions(-) create mode 100644 tests/regression/issue257_remove_shell_fallback_test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f1e7f2..e3fe825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ --- develop --- +* issue#257: Remove /bin/sh fallback from alert command execution path * 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..db946d1 100644 --- a/functions.php +++ b/functions.php @@ -1525,12 +1525,15 @@ function syslog_process_alert($alert, $sql, $params, $count, $hostname = '') { $logMessage = "SYSLOG NOTICE: Executing '$command'"; - $cparts = explode(' ', $command); + $cparts = preg_split('/\s+/', trim($command)); + $output = array(); + $returnCode = 0; - if (is_executable($cparts[0])) { + if (cacti_sizeof($cparts) && is_executable($cparts[0])) { exec($command, $output, $returnCode); } else { - exec('/bin/sh ' . $command, $output, $returnCode); + $returnCode = 126; + cacti_log("SYSLOG ERROR: Alert command is not executable: '$command'", false, 'SYSTEM'); } // Append the return code to the log message without the dot @@ -1585,12 +1588,15 @@ function syslog_process_alert($alert, $sql, $params, $count, $hostname = '') { $logMessage = "SYSLOG NOTICE: Executing '$command'"; - $cparts = explode(' ', $command); + $cparts = preg_split('/\s+/', trim($command)); + $output = array(); + $returnCode = 0; - if (is_executable($cparts[0])) { + if (cacti_sizeof($cparts) && is_executable($cparts[0])) { exec($command, $output, $returnCode); } else { - exec('/bin/sh ' . $command, $output, $returnCode); + $returnCode = 126; + cacti_log("SYSLOG ERROR: Alert command is not executable: '$command'", false, 'SYSTEM'); } // Append the return code to the log message without the dot @@ -2421,4 +2427,3 @@ function alert_replace_variables($alert, $results, $hostname = '') { return $command; } - diff --git a/tests/regression/issue257_remove_shell_fallback_test.php b/tests/regression/issue257_remove_shell_fallback_test.php new file mode 100644 index 0000000..9cbda8f --- /dev/null +++ b/tests/regression/issue257_remove_shell_fallback_test.php @@ -0,0 +1,30 @@ + Date: Fri, 6 Mar 2026 23:06:04 -0800 Subject: [PATCH 2/4] fix: move execute log inside is_executable guard; strip quotes from path Two issues with the command execution paths: 1. The 'Executing' log message was emitted even on the 126 (not executable) path because the log call was after the if/else block. Move the cacti_log call inside the if branch so it only fires when the command actually runs. 2. preg_split on whitespace yields $cparts[0] = '"path' for quoted paths like "/usr/bin/cmd" arg. Introduce $executable = trim($cparts[0], '"\\') so is_executable() sees a clean path. Also tighten regression test to use preg_match_all() patterns instead of exact substr_count() strings, which break on minor whitespace drift. Signed-off-by: Thomas Vincent --- functions.php | 32 ++++++------------- .../issue257_remove_shell_fallback_test.php | 6 ++-- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/functions.php b/functions.php index db946d1..d8badee 100644 --- a/functions.php +++ b/functions.php @@ -1521,26 +1521,20 @@ function syslog_process_alert($alert, $sql, $params, $count, $hostname = '') { } if (trim($alert['command']) != '' && !$found) { - $command = alert_replace_variables($alert, $results, $hostname); - - $logMessage = "SYSLOG NOTICE: Executing '$command'"; - + $command = alert_replace_variables($alert, $results, $hostname); $cparts = preg_split('/\s+/', trim($command)); + /* trim surrounding quotes so paths like "/usr/bin/cmd" resolve correctly */ + $executable = trim($cparts[0], '"\''); $output = array(); $returnCode = 0; - if (cacti_sizeof($cparts) && is_executable($cparts[0])) { + if (cacti_sizeof($cparts) && is_executable($executable)) { exec($command, $output, $returnCode); + cacti_log("SYSLOG NOTICE: Executing '$command' Command return code: $returnCode", true, 'SYSTEM'); } else { $returnCode = 126; cacti_log("SYSLOG ERROR: Alert command is not executable: '$command'", false, 'SYSTEM'); } - - // Append the return code to the log message without the dot - $logMessage .= " Command return code: $returnCode"; - - // Log the combined message - cacti_log($logMessage, true, 'SYSTEM'); } } @@ -1584,26 +1578,20 @@ function syslog_process_alert($alert, $sql, $params, $count, $hostname = '') { } if (trim($alert['command']) != '' && !$found) { - $command = alert_replace_variables($alert, $results, $hostname); - - $logMessage = "SYSLOG NOTICE: Executing '$command'"; - + $command = alert_replace_variables($alert, $results, $hostname); $cparts = preg_split('/\s+/', trim($command)); + /* trim surrounding quotes so paths like "/usr/bin/cmd" resolve correctly */ + $executable = trim($cparts[0], '"\''); $output = array(); $returnCode = 0; - if (cacti_sizeof($cparts) && is_executable($cparts[0])) { + if (cacti_sizeof($cparts) && is_executable($executable)) { exec($command, $output, $returnCode); + cacti_log("SYSLOG NOTICE: Executing '$command' Command return code: $returnCode", true, 'SYSTEM'); } else { $returnCode = 126; cacti_log("SYSLOG ERROR: Alert command is not executable: '$command'", false, 'SYSTEM'); } - - // Append the return code to the log message without the dot - $logMessage .= " Command return code: $returnCode"; - - // Log the combined message - cacti_log($logMessage, true, 'SYSTEM'); } } } diff --git a/tests/regression/issue257_remove_shell_fallback_test.php b/tests/regression/issue257_remove_shell_fallback_test.php index 9cbda8f..3cb5a84 100644 --- a/tests/regression/issue257_remove_shell_fallback_test.php +++ b/tests/regression/issue257_remove_shell_fallback_test.php @@ -12,17 +12,17 @@ exit(1); } -if (substr_count($functions, "preg_split('/\\s+/', trim(\$command))") < 2) { +if (preg_match_all('/\bpreg_split\b/', $functions) < 2) { fwrite(STDERR, "Expected command tokenization update is missing.\n"); exit(1); } -if (substr_count($functions, '$returnCode = 126;') < 2) { +if (preg_match_all('/\$returnCode\s*=\s*126/', $functions) < 2) { fwrite(STDERR, "Expected non-executable return code handling is missing.\n"); exit(1); } -if (substr_count($functions, "SYSLOG ERROR: Alert command is not executable: '\$command'") < 2) { +if (preg_match_all('/SYSLOG ERROR: Alert command is not executable/', $functions) < 2) { fwrite(STDERR, "Expected non-executable command log message is missing.\n"); exit(1); } From a9a887e2dce4dc58da0b44a80da837a35c4f3944 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sat, 7 Mar 2026 05:20:58 -0800 Subject: [PATCH 3/4] fix: distinguish PATH-lookup from missing/non-executable in alert command error The 'not executable' log message fires for two different reasons: the command uses a bare name without a path separator (PATH-based lookup, which is intentionally unsupported) or the resolved path does not exist or lacks the executable bit. Emit the reason so operators can diagnose their alert configuration without reading source. Closes #6785-adjacent (no new issue; improvement to existing fix) Signed-off-by: Thomas Vincent Signed-off-by: Thomas Vincent --- functions.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/functions.php b/functions.php index d8badee..214b52e 100644 --- a/functions.php +++ b/functions.php @@ -1533,7 +1533,10 @@ function syslog_process_alert($alert, $sql, $params, $count, $hostname = '') { cacti_log("SYSLOG NOTICE: Executing '$command' Command return code: $returnCode", true, 'SYSTEM'); } else { $returnCode = 126; - cacti_log("SYSLOG ERROR: Alert command is not executable: '$command'", false, 'SYSTEM'); + $reason = (strpos($executable, DIRECTORY_SEPARATOR) === false) + ? 'PATH-based lookups are not supported; use an absolute path' + : 'file not found or not marked executable'; + cacti_log("SYSLOG ERROR: Alert command is not executable: '$command' -- $reason", false, 'SYSTEM'); } } @@ -1590,7 +1593,10 @@ function syslog_process_alert($alert, $sql, $params, $count, $hostname = '') { cacti_log("SYSLOG NOTICE: Executing '$command' Command return code: $returnCode", true, 'SYSTEM'); } else { $returnCode = 126; - cacti_log("SYSLOG ERROR: Alert command is not executable: '$command'", false, 'SYSTEM'); + $reason = (strpos($executable, DIRECTORY_SEPARATOR) === false) + ? 'PATH-based lookups are not supported; use an absolute path' + : 'file not found or not marked executable'; + cacti_log("SYSLOG ERROR: Alert command is not executable: '$command' -- $reason", false, 'SYSTEM'); } } } From 818dfc0ec227ff89cb2cf0e25ab611c2b86bdaab Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 8 Mar 2026 03:55:28 -0700 Subject: [PATCH 4/4] 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 e3fe825..c7fd613 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ --- develop --- -* issue#257: Remove /bin/sh fallback from alert command execution path +* issue#257: Remove /bin/sh fallback execution path for alert commands * 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