fix: allowlist path traversal guard, MIME validation, and XSS test robustness

- Add strpos '..' guard after redirect URL allowlist to block same-origin
  path traversal (a/../syslog_alerts.php?action=purge bypassed allowlist)
- Add urldecode() before allowlist so percent-encoded characters are
  normalised before pattern matching
- Add mime_content_type() check on uploaded XML files to reject non-XML
  content before file_get_contents; fopen guard already present
- issue252_xss_output_test: replace file_get_contents with
  php_strip_whitespace() for PHP files so commented-out html_escape()
  calls cannot satisfy the assertion and mask a real XSS regression
- Partition pruning: break on invalid PARTITION_NAME instead of continue;
  partitions are age-ordered so an invalid name means subsequent entries
  cannot be safely dropped

Author: somethingwithproof

functions.php

@@ -182,13 +182,21 @@ function syslog_apply_selected_items_action($selected_items, $drp_action, $actio
 }
 
 function syslog_get_import_xml_payload($redirect_url) {
+	/* Decode percent-encoding before allowlist validation so that encoded
+	   characters (e.g. %2F, %3A) cannot smuggle past the regex. */
+	$redirect_url = urldecode($redirect_url);
+
 	/* Allow only relative paths that start with a filename character. Schemes
 	   (http://), protocol-relative URLs (//), and backslash prefixes (\, /\)
 	   all fail to match the allowlist and collapse to the safe default. */
 	if (!preg_match('/^[a-zA-Z0-9_\-][a-zA-Z0-9_\-\.\/]*(?:\?[a-zA-Z0-9_\-&=%\.+]*)?$/', $redirect_url)) {
 		$redirect_url = 'index.php';
 	}
 
+	if (strpos($redirect_url, '..') !== false) {
+		$redirect_url = 'index.php';
+	}
+
 	if (trim(get_nfilter_request_var('import_text')) != '') {
 		/* textbox input */
 		return get_nfilter_request_var('import_text');
@@ -216,6 +224,15 @@ function syslog_get_import_xml_payload($redirect_url) {
 			exit;
 		}
 
+		/* Reject non-XML uploads based on MIME sniffing of the actual bytes,
+		   not the browser-supplied Content-Type which is attacker-controlled. */
+		$mime = function_exists('mime_content_type') ? mime_content_type($tmp_name) : '';
+		if ($mime !== 'text/xml' && $mime !== 'application/xml') {
+			cacti_log('SYSLOG ERROR: Uploaded import file is not XML (detected: ' . $mime . ')', false, 'SYSTEM');
+			header('Location: ' . $redirect_url);
+			exit;
+		}
+
 		$fp = fopen($tmp_name, 'rb');
 
 		if ($fp === false) {
@@ -433,11 +450,11 @@ function syslog_partition_remove($table) {
 					   binding for DDL statements. */
 					if (!preg_match('/^d\d{8}$/', $oldest['PARTITION_NAME'])) {
 						cacti_log("SYSLOG ERROR: Unexpected partition name format '" . $oldest['PARTITION_NAME'] . "' for table '$table', skipping, cannot prune past this entry", false, 'SYSTEM');
-						$i++;
-						/* Do NOT decrement $user_partitions: no partition was dropped,
-						   so the actual count is unchanged. The upper bound on $i
-						   prevents an infinite loop when all remaining names are invalid. */
-						continue;
+						/* Stop immediately: partitions are ordered by age, so an invalid
+						   name means we cannot safely drop any further entries. Breaking
+						   here also ensures the loop terminates even if all remaining
+						   names are invalid. */
+						break;
 					}
 
 					cacti_log("SYSLOG: Removing old partition '" . $oldest['PARTITION_NAME'] . "' from table '$table'", false, 'SYSTEM');

tests/regression/issue252_xss_output_test.php

@@ -1,14 +1,25 @@
 <?php
 
-$syslogPhp   = file_get_contents(dirname(__DIR__, 2) . '/syslog.php');
-$reportsPhp  = file_get_contents(dirname(__DIR__, 2) . '/syslog_reports.php');
-$removalPhp  = file_get_contents(dirname(__DIR__, 2) . '/syslog_removal.php');
-$functionsJs = file_get_contents(dirname(__DIR__, 2) . '/js/functions.js');
-
-if ($syslogPhp === false || $reportsPhp === false || $removalPhp === false || $functionsJs === false) {
-	fwrite(STDERR, "Failed to load one or more plugin files for issue252 checks.\n");
-	exit(1);
-}
+$syslog_path  = dirname(__DIR__, 2) . '/syslog.php';
+$reports_path = dirname(__DIR__, 2) . '/syslog_reports.php';
+$removal_path = dirname(__DIR__, 2) . '/syslog_removal.php';
+$alerts_path  = dirname(__DIR__, 2) . '/syslog_alerts.php';
+$functions_js = dirname(__DIR__, 2) . '/js/functions.js';
+
+foreach ([$syslog_path, $reports_path, $removal_path, $alerts_path, $functions_js] as $path) {
+	if (!file_exists($path)) {
+		fwrite(STDERR, "Failed to load required file for issue252 checks: $path\n");
+		exit(1);
+	}
+}
+
+/* php_strip_whitespace() removes all PHP comments before asserting, so a
+   commented-out html_escape() call cannot satisfy the check and mask XSS. */
+$syslogPhp   = php_strip_whitespace($syslog_path);
+$reportsPhp  = php_strip_whitespace($reports_path);
+$removalPhp  = php_strip_whitespace($removal_path);
+$alertsPhp   = php_strip_whitespace($alerts_path);
+$functionsJs = file_get_contents($functions_js);
 
 if (substr_count($syslogPhp, "html_escape(\$host['host'])") < 2) {
 	fwrite(STDERR, "Expected escaped host rendering in syslog.php output paths.\n");
@@ -25,13 +36,6 @@
 	exit(1);
 }
 
-$alertsPhp = file_get_contents(dirname(__DIR__, 2) . '/syslog_alerts.php');
-
-if ($alertsPhp === false) {
-	fwrite(STDERR, "Failed to load syslog_alerts.php for issue252 checks.\n");
-	exit(1);
-}
-
 if (strpos($alertsPhp, "html_escape(\$alert_info)") === false) {
 	fwrite(STDERR, "Expected escaped alert confirmation list entries.\n");
 	exit(1);