From 44e016bf1b248d249fa1be224b01c1c092be4c22 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Thu, 27 Aug 2026 20:32:34 -0700 Subject: [PATCH 1/2] Harden Syslog CSV exports and imports --- CHANGELOG.md | 1 + functions.php | 71 ++++++--- ...issue256_262_csv_import_hardening_test.php | 144 ++++++++++++++++++ .../issue277_import_payload_loader_test.php | 5 +- 4 files changed, 199 insertions(+), 22 deletions(-) create mode 100644 tests/regression/issue256_262_csv_import_hardening_test.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a22b427..57a81ef7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ * issue#256: hardening: prevent CSV formula injection and malformed CSV output in exports * issue#258: Execute CREATE TABLE SQL correctly during replication sync * issue#260: hardening: replace eval-based callback execution in syslog autocomplete JS +* issue#262: Harden CSV exports and XML import payload handling * 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) * issue#298: syslog poller: lock timeout, signal handler, and earlier partition rotation diff --git a/functions.php b/functions.php index 1e54c132..06d0a7eb 100644 --- a/functions.php +++ b/functions.php @@ -144,15 +144,25 @@ 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']) && - $_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']; @@ -166,6 +176,14 @@ function syslog_get_import_xml_payload($redirect_url) { exit; } + $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 +192,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 +208,22 @@ function syslog_get_import_xml_payload($redirect_url) { exit; } +function syslog_csv_cell(mixed $value): string { + $value = (string) $value; + + if ($value === '' || str_starts_with($value, "'")) { + return $value; + } + + $trimmed = ltrim($value, ' '); + + if ($trimmed !== '' && in_array($trimmed[0], ['=', '+', '-', '@', "\t", "\r"], true)) { + return "'" . $value; + } + + return $value; +} + function syslog_is_partitioned() { global $syslogdb_default; @@ -828,7 +862,7 @@ function syslog_export($tab) { $line = ['host', 'facility', 'priority', 'program', 'date', 'message']; - fputcsv($fp, $line); + fputcsv($fp, $line, ',', '"', ''); if (cacti_sizeof($messages)) { foreach ($messages as $message) { @@ -851,23 +885,23 @@ function syslog_export($tab) { } if (isset($hosts[$message['host_id']])) { - $host = trim($hosts[$message['host_id']], ' =+-@'); + $host = $hosts[$message['host_id']]; } else { $host = 'Unknown'; } - $logmsg = trim($message[$syslog_incoming_config['textField']], ' =+-@'); + $logmsg = $message[$syslog_incoming_config['textField']]; - $line = [ + $line = array_map('syslog_csv_cell', [ $host, ucfirst($facility), ucfirst($priority), ucfirst($program), $message['logtime'], $logmsg - ]; + ]); - fputcsv($fp, $line); + fputcsv($fp, $line, ',', '"', ''); } } @@ -884,7 +918,7 @@ function syslog_export($tab) { $fp = fopen('php://output', 'w'); - fputcsv($fp, $line); + fputcsv($fp, $line, ',', '"', ''); if (cacti_sizeof($messages)) { foreach ($messages as $message) { @@ -894,21 +928,18 @@ function syslog_export($tab) { $severity = 'Unknown'; } - $host = trim($message['host'], ' =+-@'); - $logmsg = trim($message['logmsg'], ' =+-@'); - - $line = [ + $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'] - ]; + ]); - fputcsv($fp, $line); + fputcsv($fp, $line, ',', '"', ''); } } diff --git a/tests/regression/issue256_262_csv_import_hardening_test.php b/tests/regression/issue256_262_csv_import_hardening_test.php new file mode 100644 index 00000000..5e7ea130 --- /dev/null +++ b/tests/regression/issue256_262_csv_import_hardening_test.php @@ -0,0 +1,144 @@ + SYSLOG_IMPORT_MAX_BYTES', + '$size <= 0 || $size > SYSLOG_IMPORT_MAX_BYTES', + 'function syslog_csv_cell(mixed $value): string', + "array_map('syslog_csv_cell'", +] as $needle) { + if (!str_contains($functions, $needle)) { + fwrite(STDERR, "Missing import/export hardening: $needle\n"); + exit(1); + } +} + +if (substr_count($functions, "array_map('syslog_csv_cell'") !== 2) { + fwrite(STDERR, "Both Syslog CSV export paths must harden every cell\n"); + exit(1); +} + +if (substr_count($functions, 'fputcsv($fp, $line, \',\', \'"\', \'\')') !== 4) { + fwrite(STDERR, "Every CSV write must disable the proprietary backslash escape\n"); + exit(1); +} + +if (str_contains($functions, 'trim($hosts[$message[\'host_id\']], \' =+-@\')') || + str_contains($functions, 'trim($message[$syslog_incoming_config[\'textField\']], \' =+-@\')')) { + fwrite(STDERR, "CSV hardening must not mutate exported message data\n"); + exit(1); +} + +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 issue256_262_csv_cell', $match[0])); + +foreach ([ + ['=SUM(A1)', "'=SUM(A1)"], + ["\tevil", "'\tevil"], + ["\revil", "'\revil"], + [' =SUM(A1)', "' =SUM(A1)"], + [" \t=SUM(A1)", "' \t=SUM(A1)"], + [' ', ' '], + ["'=SUM(A1)", "'=SUM(A1)"], + ['router-01', 'router-01'], +] as [$input, $expected]) { + if (issue256_262_csv_cell($input) !== $expected) { + fwrite(STDERR, 'CSV formula hardening failed for ' . var_export($input, true) . "\n"); + exit(1); + } +} + +$input = [ + 'router-01', + 'attack\\",=cmd|\'/c calc\'!A0,"x', + 'tail', +]; +$safe = array_map('issue256_262_csv_cell', $input); +$csv = fopen('php://temp', 'w+'); + +if ($csv === false) { + fwrite(STDERR, "Unable to open CSV regression stream\n"); + exit(1); +} + +fputcsv($csv, $safe, ',', '"', ''); +rewind($csv); +$row = stream_get_contents($csv); +fclose($csv); + +if ($row === false) { + fwrite(STDERR, "Unable to read CSV regression stream\n"); + exit(1); +} + +$parsed = str_getcsv(rtrim($row, "\r\n"), ',', '"', ''); + +if ($parsed !== $safe || count($parsed) !== count($input)) { + fwrite(STDERR, "CSV round trip created attacker-controlled extra cells\n"); + exit(1); +} + +foreach ($parsed as $cell) { + if (preg_match('/^[=+\-@\t\r]/', ltrim($cell, ' ')) === 1) { + fwrite(STDERR, "CSV round trip produced an unsafe formula-leading cell\n"); + exit(1); + } +} + +$root = dirname(__DIR__, 2); +$code = sprintf(<<<'PHP' +$payload = str_repeat('x', (5 * 1024 * 1024) + 1); + +function get_nfilter_request_var(string $name): string { + global $payload; + + return $payload; +} + +function cacti_log(string $message, bool $output, string $facility): void { + print $message; +} + +require %s; +syslog_get_import_xml_payload('/blocked'); +print 'UNREACHABLE'; +PHP, + var_export($root . '/functions.php', true) +); + +$pipes = []; +$process = proc_open([PHP_BINARY, '-r', $code], [ + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], +], $pipes); + +if (!is_resource($process)) { + fwrite(STDERR, "Unable to start oversized import regression process\n"); + exit(1); +} + +$stdout = stream_get_contents($pipes[1]); +$stderr = stream_get_contents($pipes[2]); +fclose($pipes[1]); +fclose($pipes[2]); +$status = proc_close($process); + +if ($status !== 0 || !str_contains($stdout, 'Text import payload exceeds the maximum size') || + str_contains($stdout, 'UNREACHABLE')) { + fwrite(STDERR, "Oversized text import did not fail closed: $stderr\n"); + exit(1); +} + +print "issue256_262_csv_import_hardening_test passed\n"; diff --git a/tests/regression/issue277_import_payload_loader_test.php b/tests/regression/issue277_import_payload_loader_test.php index e77ba1da..c1f9a14a 100644 --- a/tests/regression/issue277_import_payload_loader_test.php +++ b/tests/regression/issue277_import_payload_loader_test.php @@ -31,12 +31,13 @@ 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, '$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); } From d3b5537e43778effbce45289b2b675959fbe126b Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 30 Aug 2026 18:07:01 -0700 Subject: [PATCH 2/2] ci: test PHP 8.2-8.4 against Cacti and add ondrej PPA for apache mod Cacti core now requires PHP >= 8.2, so the 8.1 integration job fails the composer platform check; Ubuntu Noble also lacks libapache2-mod-php for non-native versions without the ondrej PPA. Signed-off-by: Thomas Vincent --- .github/workflows/plugin-ci-workflow.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/plugin-ci-workflow.yml b/.github/workflows/plugin-ci-workflow.yml index 86237dcf..f24a1253 100644 --- a/.github/workflows/plugin-ci-workflow.yml +++ b/.github/workflows/plugin-ci-workflow.yml @@ -96,7 +96,7 @@ jobs: strategy: fail-fast: false matrix: - php: ['8.1', '8.2', '8.3'] + php: ['8.2', '8.3'] os: [ubuntu-latest] services: @@ -140,7 +140,9 @@ jobs: run: php -v - name: Run apt-get update - run: sudo apt-get update + run: | + sudo add-apt-repository -y ppa:ondrej/php + sudo apt-get update - name: Install System Dependencies run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php${{ matrix.php }}