hardening: escape syslog output surfaces and harden host option rendering

.github/workflows/plugin-ci-workflow.yml

@@ -187,6 +187,16 @@ jobs:
           echo "Syntax errors found!"
           exit 1
         fi
+
+    - name: Run Plugin Regression Tests
+      run: |
+        cd ${{ github.workspace }}/cacti/plugins/syslog
+        if [ -d tests/regression ]; then
+          for test in tests/regression/*.php; do
+            [ -f "$test" ] || continue
+            php "$test"
+          done
+        fi
 
 
     - name: Run Cacti Poller

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 --- develop ---
 
+* issue#252: Escape syslog output surfaces and harden host option rendering
 * issue: Making changes to support Cacti 1.3
 * issue: Don't use MyISAM for non-analytical tables
 * issue: The install advisor for Syslog was broken in current Cacti releases

js/functions.js

@@ -220,7 +220,11 @@ function initSyslogMain(config) {
 
 							$.each(data, function(index, hostData) {
 								if ($('#host option[value="'+index+'"]').length == 0) {
-									$('#host').append('<option class="'+hostData.class+'" value="'+index+'">'+hostData.host+'</option>');
+									var option = $('<option>')
+										.addClass(hostData.class || '')
+										.val(index)
+										.text(hostData.host);
+									$('#host').append(option);
 								}
 							});
 

syslog.php

@@ -364,10 +364,10 @@ function syslog_statistics() {
 
 			form_alternate_row('line' . $i);
 
-			print '<td>' . (get_request_var('host') != '-2' ? $r['host']:'-') . '</td>';
-			print '<td>' . (get_request_var('facility') != '-2' ? ucfirst($r['facility']):'-') . '</td>';
-			print '<td>' . (get_request_var('priority') != '-2' ? ucfirst($r['priority']):'-') . '</td>';
-			print '<td>' . (get_request_var('program') != '-2' ? ucfirst($r['program']):'-') . '</td>';
+			print '<td>' . (get_request_var('host') != '-2' ? html_escape($r['host']):'-') . '</td>';
+			print '<td>' . (get_request_var('facility') != '-2' ? html_escape(ucfirst($r['facility'])):'-') . '</td>';
+			print '<td>' . (get_request_var('priority') != '-2' ? html_escape(ucfirst($r['priority'])):'-') . '</td>';
+			print '<td>' . (get_request_var('program') != '-2' ? html_escape(ucfirst($r['program'])):'-') . '</td>';
 			//print '<td class="right">' . $r['insert_time'] . '</td>';
 			print '<td class="right">' . $time . '</td>';
 			print '<td class="right">' . number_format_i18n($r['records'], -1)     . '</td>';
@@ -521,7 +521,7 @@ function syslog_stats_filter() {
 										$class = 'deviceUp';
 									}
 
-									print '<option class="' . $class . '" value="' . $host['host_id'] . '"'; if (get_request_var('host') == $host['host_id']) { print ' selected'; } print '>' . $host['host'] . '</option>';
+									print '<option class="' . $class . '" value="' . $host['host_id'] . '"'; if (get_request_var('host') == $host['host_id']) { print ' selected'; } print '>' . html_escape($host['host']) . '</option>';
 								}
 							}
 							?>
@@ -541,7 +541,7 @@ function syslog_stats_filter() {
 
 							if (cacti_sizeof($facilities)) {
 								foreach ($facilities as $r) {
-									print '<option value="' . $r['facility_id'] . '"'; if (get_request_var('facility') == $r['facility_id']) { print ' selected'; } print '>' . ucfirst($r['facility']) . "</option>\n";
+									print '<option value="' . $r['facility_id'] . '"'; if (get_request_var('facility') == $r['facility_id']) { print ' selected'; } print '>' . html_escape(ucfirst($r['facility'])) . "</option>\n";
 								}
 							}
 							?>
@@ -561,7 +561,7 @@ function syslog_stats_filter() {
 
 							if (cacti_sizeof($priorities)) {
 								foreach ($priorities as $r) {
-									print '<option value="' . $r['priority_id'] . '"'; if (get_request_var('priority') == $r['priority_id']) { print ' selected'; } print '>' . ucfirst($r['priority']) . "</option>\n";
+									print '<option value="' . $r['priority_id'] . '"'; if (get_request_var('priority') == $r['priority_id']) { print ' selected'; } print '>' . html_escape(ucfirst($r['priority'])) . "</option>\n";
 								}
 							}
 							?>
@@ -1349,7 +1349,7 @@ function syslog_filter($sql_where, $tab) {
 											}
 										}
 										print '>';
-										print $host['host'] . '</option>';
+										print html_escape($host['host']) . '</option>';
 									}
 								}
 								?>
@@ -2037,4 +2037,3 @@ function syslog_form_callback($form_name, $classic_sql, $column_display, $column
 		<?php
 	}
 }
-

syslog_removal.php

@@ -169,7 +169,7 @@ function form_actions() {
 				FROM `" . $syslogdb_default . "`.`syslog_remove`
 				WHERE id=" . $matches[1]);
 
-			$removal_list  .= '<li>' . $removal_info . '</li>';
+			$removal_list  .= '<li>' . html_escape($removal_info) . '</li>';
 			$removal_array[] = $matches[1];
 		}
 	}
@@ -810,4 +810,3 @@ function removal_import() {
 
 	header('Location: syslog_removal.php');
 }
-

syslog_reports.php

@@ -157,7 +157,7 @@ function form_actions() {
 				FROM `' . $syslogdb_default . '`.`syslog_reports`
 				WHERE id=' . $matches[1]);
 
-			$report_list  .= '<li>' . $report_info . '</li>';
+			$report_list  .= '<li>' . html_escape($report_info) . '</li>';
 			$report_array[] = $matches[1];
 		}
 	}
@@ -872,4 +872,3 @@ function report_import() {
 
 	header('Location: syslog_reports.php');
 }
-

tests/regression/issue252_xss_output_test.php

@@ -0,0 +1,45 @@
+<?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);
+}
+
+if (substr_count($syslogPhp, "html_escape(\$host['host'])") < 2) {
+	fwrite(STDERR, "Expected escaped host rendering in syslog.php output paths.\n");
+	exit(1);
+}
+
+if (strpos($reportsPhp, "html_escape(\$report_info)") === false) {
+	fwrite(STDERR, "Expected escaped report confirmation list entries.\n");
+	exit(1);
+}
+
+if (strpos($removalPhp, "html_escape(\$removal_info)") === false) {
+	fwrite(STDERR, "Expected escaped removal confirmation list entries.\n");
+	exit(1);
+}
+
+if (strpos($reportsPhp, "form_selectable_ecell(\$report['message'], \$report['id']);") === false) {
+	fwrite(STDERR, "Expected escaped report message cell rendering.\n");
+	exit(1);
+}
+
+if (strpos($functionsJs, "var option = $('<option>')") === false ||
+	strpos($functionsJs, ".text(hostData.host);") === false ||
+	strpos($functionsJs, "$('#host').append(option);") === false) {
+	fwrite(STDERR, "Expected DOM-safe host option rendering in js/functions.js.\n");
+	exit(1);
+}
+
+if (strpos($functionsJs, "$('#host').append('<option class=\"'+hostData.class+'\" value=\"'+index+'\">'+hostData.host+'</option>');") !== false) {
+	fwrite(STDERR, "Legacy unsafe host option HTML concatenation still present.\n");
+	exit(1);
+}
+
+echo "issue252_xss_output_test passed\n";