From a9278ebd085415e4e4bbfd2ad5eddbd759977053 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 13 Jul 2026 22:01:07 -0700 Subject: [PATCH 1/5] Harden Syslog output and imports --- functions.php | 26 +++++++++++++++-- syslog.php | 2 +- .../issue318_output_import_hardening_test.php | 28 +++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) create mode 100644 tests/regression/issue318_output_import_hardening_test.php diff --git a/functions.php b/functions.php index 1e54c13..f45e39d 100644 --- a/functions.php +++ b/functions.php @@ -144,6 +144,8 @@ function syslog_sendemail($to, $from, $subject, $message, $smsmessage = '') { } } +define('SYSLOG_IMPORT_MAX_BYTES', 5242880); + function syslog_get_import_xml_payload($redirect_url) { if (trim(get_nfilter_request_var('import_text')) != '') { // textbox input @@ -166,6 +168,13 @@ function syslog_get_import_xml_payload($redirect_url) { exit; } + $size = isset($_FILES['import_file']['size']) ? (int) $_FILES['import_file']['size'] : filesize($tmp_name); + if ($size <= 0 || $size > SYSLOG_IMPORT_MAX_BYTES) { + cacti_log('SYSLOG ERROR: Uploaded import file has an invalid size', false, 'SYSTEM'); + header('Location: ' . $redirect_url); + exit; + } + $fp = fopen($tmp_name, 'rb'); if ($fp === false) { @@ -174,7 +183,7 @@ function syslog_get_import_xml_payload($redirect_url) { exit; } - $xml_data = fread($fp, filesize($tmp_name)); + $xml_data = fread($fp, $size); fclose($fp); if ($xml_data === false) { @@ -190,6 +199,17 @@ function syslog_get_import_xml_payload($redirect_url) { exit; } +function syslog_csv_cell($value) { + $value = (string) $value; + $trimmed = ltrim($value, " \t\r\n"); + + if ($trimmed !== '' && in_array($trimmed[0], ['=', '+', '-', '@'], true)) { + return "'" . $value; + } + + return $value; +} + function syslog_is_partitioned() { global $syslogdb_default; @@ -897,7 +917,7 @@ function syslog_export($tab) { $host = trim($message['host'], ' =+-@'); $logmsg = trim($message['logmsg'], ' =+-@'); - $line = [ + $line = array_map('syslog_csv_cell', [ $message['name'], $severity, $message['logtime'], @@ -906,7 +926,7 @@ function syslog_export($tab) { ucfirst($message['facility']), ucfirst($message['priority']), $message['count'] - ]; + ]); fputcsv($fp, $line); } diff --git a/syslog.php b/syslog.php index 0d1c45c..2429ff3 100644 --- a/syslog.php +++ b/syslog.php @@ -219,7 +219,7 @@ function syslog_view_alarm() { WHERE seq = ?", [get_request_var('id')]); - print trim($html, "' "); + print nl2br(html_escape(trim($html, "' "))); print ''; diff --git a/tests/regression/issue318_output_import_hardening_test.php b/tests/regression/issue318_output_import_hardening_test.php new file mode 100644 index 0000000..36d9870 --- /dev/null +++ b/tests/regression/issue318_output_import_hardening_test.php @@ -0,0 +1,28 @@ + SYSLOG_IMPORT_MAX_BYTES', + 'function syslog_csv_cell($value)', + "array_map('syslog_csv_cell'", +] as $needle) { + if (strpos($functions, $needle) === false) { + fwrite(STDERR, "Missing import/export hardening: $needle\n"); + exit(1); + } +} + +if (strpos($syslog, 'nl2br(html_escape(trim($html, "\' ")))') === false) { + fwrite(STDERR, "Alert viewer must escape stored HTML\n"); + exit(1); +} + +echo "issue318_output_import_hardening_test passed\n"; From a7b4522060048edf03e0a8a039c9dc81727e0bba Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Tue, 14 Jul 2026 02:36:26 -0700 Subject: [PATCH 2/5] Use runner PHP without versioned Apache package --- .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 86237dc..3840fa2 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 c64f592a019feedfc7c232fcc549c2cb93d10d7e Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 16 Aug 2026 23:17:31 -0700 Subject: [PATCH 3/5] 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 | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 86237dc..265b555 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 @@ -143,7 +145,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 libapache2-mod-php - name: Start SNMPD Agent and Test run: | @@ -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 f013efd1d7465a124cc7a60c2d6885ed1947b99e Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 00:25:31 -0700 Subject: [PATCH 4/5] Modernize output hardening for PHP 8.0 --- functions.php | 29 +++++++++-------- .../issue277_import_payload_loader_test.php | 4 +-- .../issue318_output_import_hardening_test.php | 31 ++++++++++++++++--- 3 files changed, 44 insertions(+), 20 deletions(-) diff --git a/functions.php b/functions.php index f45e39d..cf4b5ea 100644 --- a/functions.php +++ b/functions.php @@ -144,17 +144,17 @@ function syslog_sendemail($to, $from, $subject, $message, $smsmessage = '') { } } -define('SYSLOG_IMPORT_MAX_BYTES', 5242880); +const SYSLOG_IMPORT_MAX_BYTES = 5 * 1024 * 1024; function syslog_get_import_xml_payload($redirect_url) { - if (trim(get_nfilter_request_var('import_text')) != '') { + if (trim(get_nfilter_request_var('import_text')) !== '') { // textbox input return get_nfilter_request_var('import_text'); } if (isset($_FILES['import_file']['tmp_name']) && - $_FILES['import_file']['tmp_name'] != 'none' && - $_FILES['import_file']['tmp_name'] != '') { + $_FILES['import_file']['tmp_name'] !== 'none' && + $_FILES['import_file']['tmp_name'] !== '') { // file upload $tmp_name = $_FILES['import_file']['tmp_name']; @@ -168,7 +168,8 @@ function syslog_get_import_xml_payload($redirect_url) { exit; } - $size = isset($_FILES['import_file']['size']) ? (int) $_FILES['import_file']['size'] : filesize($tmp_name); + $size = (int) ($_FILES['import_file']['size'] ?? filesize($tmp_name)); + if ($size <= 0 || $size > SYSLOG_IMPORT_MAX_BYTES) { cacti_log('SYSLOG ERROR: Uploaded import file has an invalid size', false, 'SYSTEM'); header('Location: ' . $redirect_url); @@ -199,11 +200,16 @@ function syslog_get_import_xml_payload($redirect_url) { exit; } -function syslog_csv_cell($value) { +function syslog_csv_cell(mixed $value): string { $value = (string) $value; - $trimmed = ltrim($value, " \t\r\n"); - if ($trimmed !== '' && in_array($trimmed[0], ['=', '+', '-', '@'], true)) { + if ($value === '' || str_starts_with($value, "'")) { + return $value; + } + + $trimmed = ltrim($value, ' '); + + if ($trimmed !== '' && in_array($trimmed[0], ['=', '+', '-', '@', "\t", "\r"], true)) { return "'" . $value; } @@ -914,15 +920,12 @@ function syslog_export($tab) { $severity = 'Unknown'; } - $host = trim($message['host'], ' =+-@'); - $logmsg = trim($message['logmsg'], ' =+-@'); - $line = array_map('syslog_csv_cell', [ $message['name'], $severity, $message['logtime'], - $logmsg, - $host, + $message['logmsg'], + $message['host'], ucfirst($message['facility']), ucfirst($message['priority']), $message['count'] diff --git a/tests/regression/issue277_import_payload_loader_test.php b/tests/regression/issue277_import_payload_loader_test.php index e77ba1d..eb36c9d 100644 --- a/tests/regression/issue277_import_payload_loader_test.php +++ b/tests/regression/issue277_import_payload_loader_test.php @@ -31,12 +31,12 @@ exit(1); } -if (strpos($functions, 'function syslog_get_import_xml_payload(') === false) { +if (!str_contains($functions, 'function syslog_get_import_xml_payload(')) { fwrite(STDERR, "Shared import payload loader helper is missing.\n"); exit(1); } -if (strpos($functions, "trim(get_nfilter_request_var('import_text')) != ''") === false) { +if (!str_contains($functions, "trim(get_nfilter_request_var('import_text')) !== ''")) { fwrite(STDERR, "Shared import payload loader is missing trimmed text handling.\n"); exit(1); } diff --git a/tests/regression/issue318_output_import_hardening_test.php b/tests/regression/issue318_output_import_hardening_test.php index 36d9870..5d2bf5a 100644 --- a/tests/regression/issue318_output_import_hardening_test.php +++ b/tests/regression/issue318_output_import_hardening_test.php @@ -1,7 +1,7 @@ SYSLOG_IMPORT_MAX_BYTES', - 'function syslog_csv_cell($value)', + 'function syslog_csv_cell(mixed $value): string', "array_map('syslog_csv_cell'", ] as $needle) { - if (strpos($functions, $needle) === false) { + if (!str_contains($functions, $needle)) { fwrite(STDERR, "Missing import/export hardening: $needle\n"); exit(1); } } -if (strpos($syslog, 'nl2br(html_escape(trim($html, "\' ")))') === false) { +if (!preg_match('/function\s+syslog_csv_cell\s*\([^)]*\)\s*:\s*string\s*\{.*?\n\}/s', $functions, $match)) { + fwrite(STDERR, "Unable to extract syslog_csv_cell()\n"); + exit(1); +} + +eval(str_replace('function syslog_csv_cell', 'function issue318_csv_cell', $match[0])); + +foreach ([ + ['=SUM(A1)', "'=SUM(A1)"], + ["\tevil", "'\tevil"], + ["\revil", "'\revil"], + [' =SUM(A1)', "' =SUM(A1)"], + ["'=SUM(A1)", "'=SUM(A1)"], + ['router-01', 'router-01'], +] as [$input, $expected]) { + if (issue318_csv_cell($input) !== $expected) { + fwrite(STDERR, 'CSV formula hardening failed for ' . var_export($input, true) . "\n"); + exit(1); + } +} + +if (!str_contains($syslog, 'nl2br(html_escape(trim($html, "\' ")))')) { fwrite(STDERR, "Alert viewer must escape stored HTML\n"); exit(1); } -echo "issue318_output_import_hardening_test passed\n"; +print "issue318_output_import_hardening_test passed\n"; From b71fab20dc0db19e6c2f957c310d4e750c38f23c Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 17 Aug 2026 14:51:54 -0700 Subject: [PATCH 5/5] Address remaining Copilot review findings --- .github/workflows/plugin-ci-workflow.yml | 2 +- functions.php | 12 ++++++++++-- .../issue277_import_payload_loader_test.php | 3 ++- .../issue318_output_import_hardening_test.php | 2 ++ 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 265b555..d22ab2d 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -145,7 +145,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 + run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping - name: Start SNMPD Agent and Test run: | diff --git a/functions.php b/functions.php index cf4b5ea..4452a34 100644 --- a/functions.php +++ b/functions.php @@ -147,9 +147,17 @@ function syslog_sendemail($to, $from, $subject, $message, $smsmessage = '') { const SYSLOG_IMPORT_MAX_BYTES = 5 * 1024 * 1024; function syslog_get_import_xml_payload($redirect_url) { - if (trim(get_nfilter_request_var('import_text')) !== '') { + $import_text = (string) get_nfilter_request_var('import_text'); + + if (trim($import_text) !== '') { // textbox input - return get_nfilter_request_var('import_text'); + if (strlen($import_text) > SYSLOG_IMPORT_MAX_BYTES) { + cacti_log('SYSLOG ERROR: Text import payload exceeds the maximum size', false, 'SYSTEM'); + header('Location: ' . $redirect_url); + exit; + } + + return $import_text; } if (isset($_FILES['import_file']['tmp_name']) && diff --git a/tests/regression/issue277_import_payload_loader_test.php b/tests/regression/issue277_import_payload_loader_test.php index eb36c9d..c1f9a14 100644 --- a/tests/regression/issue277_import_payload_loader_test.php +++ b/tests/regression/issue277_import_payload_loader_test.php @@ -36,7 +36,8 @@ exit(1); } -if (!str_contains($functions, "trim(get_nfilter_request_var('import_text')) !== ''")) { +if (!str_contains($functions, '$import_text = (string) get_nfilter_request_var(\'import_text\')') || + !str_contains($functions, 'trim($import_text) !== \'\'')) { fwrite(STDERR, "Shared import payload loader is missing trimmed text handling.\n"); exit(1); } diff --git a/tests/regression/issue318_output_import_hardening_test.php b/tests/regression/issue318_output_import_hardening_test.php index 5d2bf5a..465c7b0 100644 --- a/tests/regression/issue318_output_import_hardening_test.php +++ b/tests/regression/issue318_output_import_hardening_test.php @@ -10,6 +10,8 @@ foreach ([ 'SYSLOG_IMPORT_MAX_BYTES', + '$import_text = (string) get_nfilter_request_var(\'import_text\')', + 'strlen($import_text) > SYSLOG_IMPORT_MAX_BYTES', '$size <= 0 || $size > SYSLOG_IMPORT_MAX_BYTES', 'function syslog_csv_cell(mixed $value): string', "array_map('syslog_csv_cell'",