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); }