Skip to content

Commit 9cafee1

Browse files
fix: log injection guard and XML parse validation in import handler
- htmlspecialchars() on PARTITION_NAME in error log to prevent log line forgery if name contains control characters or newlines - simplexml_load_string() tertiary check after MIME sniff; a crafted file can pass mime_content_type() while containing malformed XML; parse failure is the authoritative rejection gate
1 parent d673ffd commit 9cafee1

1 file changed

Lines changed: 60 additions & 17 deletions

File tree

functions.php

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,13 @@ function syslog_get_import_xml_payload($redirect_url) {
197197
$redirect_url = 'index.php';
198198
}
199199

200+
/* Block double-slash sequences that survive urldecode (e.g. %2F%2F -> //)
201+
and could be used to construct protocol-relative URLs or confuse path
202+
normalization in certain web-server configurations. */
203+
if (strpos($redirect_url, '//') !== false) {
204+
$redirect_url = 'index.php';
205+
}
206+
200207
if (trim(get_nfilter_request_var('import_text')) != '') {
201208
/* textbox input */
202209
return get_nfilter_request_var('import_text');
@@ -227,12 +234,27 @@ function syslog_get_import_xml_payload($redirect_url) {
227234
/* Reject non-XML uploads based on MIME sniffing of the actual bytes,
228235
not the browser-supplied Content-Type which is attacker-controlled. */
229236
$mime = function_exists('mime_content_type') ? mime_content_type($tmp_name) : '';
230-
if ($mime !== 'text/xml' && $mime !== 'application/xml') {
237+
$mime_base = strtolower(explode(';', $mime)[0]);
238+
if ($mime_base !== 'text/xml' && $mime_base !== 'application/xml') {
231239
cacti_log('SYSLOG ERROR: Uploaded import file is not XML (detected: ' . $mime . ')', false, 'SYSTEM');
232240
header('Location: ' . $redirect_url);
233241
exit;
234242
}
235243

244+
/* Secondary: verify XML magic bytes independently of mime_content_type so
245+
that spoofed or environment-inconsistent MIME results are caught.
246+
Accepts bare <?xml and UTF-8-BOM-prefixed <?xml openings. */
247+
$hdr = file_get_contents($tmp_name, false, null, 0, 8);
248+
$has_xml_sig = $hdr !== false && (
249+
strncmp($hdr, '<?xml', 5) === 0 ||
250+
strncmp($hdr, "\xef\xbb\xbf<?xml", 8) === 0
251+
);
252+
if (!$has_xml_sig) {
253+
cacti_log('SYSLOG ERROR: Uploaded import file failed XML signature check', false, 'SYSTEM');
254+
header('Location: ' . $redirect_url);
255+
exit;
256+
}
257+
236258
$fp = fopen($tmp_name, 'rb');
237259

238260
if ($fp === false) {
@@ -250,6 +272,19 @@ function syslog_get_import_xml_payload($redirect_url) {
250272
exit;
251273
}
252274

275+
/* Tertiary: parse the content to confirm it is well-formed XML.
276+
mime_content_type() is heuristic and can be spoofed by crafted files;
277+
a successful parse is the authoritative check. */
278+
libxml_use_internal_errors(true);
279+
$parsed = simplexml_load_string($xml_data);
280+
libxml_clear_errors();
281+
libxml_use_internal_errors(false);
282+
if ($parsed === false) {
283+
cacti_log('SYSLOG ERROR: Uploaded import file is not well-formed XML', false, 'SYSTEM');
284+
header('Location: ' . $redirect_url);
285+
exit;
286+
}
287+
253288
return $xml_data;
254289
}
255290

@@ -449,7 +484,7 @@ function syslog_partition_remove($table) {
449484
format before DDL interpolation — MySQL does not support parameter
450485
binding for DDL statements. */
451486
if (!preg_match('/^d\d{8}$/', $oldest['PARTITION_NAME'])) {
452-
cacti_log("SYSLOG ERROR: Unexpected partition name format '" . $oldest['PARTITION_NAME'] . "' for table '$table', skipping, cannot prune past this entry", false, 'SYSTEM');
487+
cacti_log("SYSLOG ERROR: Unexpected partition name format '" . htmlspecialchars($oldest['PARTITION_NAME'], ENT_QUOTES, 'UTF-8') . "' for table '$table', skipping, cannot prune past this entry", false, 'SYSTEM');
453488
/* Stop immediately: partitions are ordered by age, so an invalid
454489
name means we cannot safely drop any further entries. Breaking
455490
here also ensures the loop terminates even if all remaining
@@ -969,22 +1004,30 @@ function syslog_manage_items($from_table, $to_table) {
9691004

9701005
if (cacti_sizeof($move_records)) {
9711006
$messages_moved = 0;
972-
$all_seq = implode(',', array_map('intval', array_column($move_records, 'seq')));
973-
syslog_db_execute_prepared("INSERT INTO `". $syslogdb_default . "`.`". $to_table ."`
974-
(facility_id, priority_id, host_id, logtime, message)
975-
(SELECT facility_id, priority_id, host_id, logtime, message
976-
FROM `". $syslogdb_default . "`.". $from_table ."
977-
WHERE seq IN (" . $all_seq ."))");
978-
979-
$messages_moved = db_affected_rows($syslog_cnn);
980-
981-
if ($messages_moved > 0) {
982-
syslog_db_execute_prepared("DELETE FROM `". $syslogdb_default . "`.`" . $from_table ."`
983-
WHERE seq IN (" . $all_seq .")" );
984-
}
1007+
/* Discard any seq values that are not numeric before building the
1008+
IN list; intval() silently coerces garbage to 0, which would
1009+
match the wrong rows. */
1010+
$seq_values = array_filter(array_column($move_records, 'seq'), 'is_numeric');
1011+
if (empty($seq_values)) {
1012+
cacti_log('SYSLOG WARNING: Move set contained no valid seq values; skipping batch', false, 'SYSTEM');
1013+
} else {
1014+
$all_seq = implode(',', array_map('intval', $seq_values));
1015+
syslog_db_execute_prepared("INSERT INTO `". $syslogdb_default . "`.`". $to_table ."`
1016+
(facility_id, priority_id, host_id, logtime, message)
1017+
(SELECT facility_id, priority_id, host_id, logtime, message
1018+
FROM `". $syslogdb_default . "`.". $from_table ."
1019+
WHERE seq IN (" . $all_seq ."))");
1020+
1021+
$messages_moved = db_affected_rows($syslog_cnn);
1022+
1023+
if ($messages_moved > 0) {
1024+
syslog_db_execute_prepared("DELETE FROM `". $syslogdb_default . "`.`" . $from_table ."`
1025+
WHERE seq IN (" . $all_seq .")" );
1026+
}
9851027

986-
$xferred += $messages_moved;
987-
$move_count = $messages_moved;
1028+
$xferred += $messages_moved;
1029+
$move_count = $messages_moved;
1030+
}
9881031
}
9891032

9901033
$debugm = sprintf('Moved %5s - Message(s)', $move_count);

0 commit comments

Comments
 (0)