QA: Merging Security Updates from 1.2.x into develop

Author: TheWitness

CHANGELOG

@@ -121,7 +121,7 @@ Cacti CHANGELOG
 -feature#5328: Search through Data Query fields when searching for Graphs in Tree
 -feature#5368: Allow all key searching calls to be delayed by defined amount
 -feature#5378: When reload page in browser with graphs created scroll window always return to top
--feature#5389: Forgot password function and email notification about new user or reset user password 
+-feature#5389: Forgot password function and email notification about new user or reset user password
 -feature#5406: Cacti connection handling should forcibly close connections to work around PDO uncertainty
 -feature#5407: Add last_updated column to the boost table to increase diagnostics in Cacti
 -feature#5416: Display device class before package import
@@ -169,12 +169,12 @@ Cacti CHANGELOG
 -feature#5970: Import/Export Automation should take THold into account
 -feature#5971: Add a method to mark missing Data in all graphs globally
 -feature#5993: Allow Automation Rules to Search through Site Information
--feature#5997: Add filter for devices in maintenance plan, add icon to device list 
+-feature#5997: Add filter for devices in maintenance plan, add icon to device list
 -feature#6001: Extend SYSTEM STATS log entry
 -feature#6033: Allow Admins to see when a template was last modified
 -feature#6034: Allow Users to Drill Up from CDEF's and VDEF's to Graphs and Graph Templates
 -feature#6044: Enable the capabilities to view historical Classical Cacti Report from the GUI
--feature#6063: Change installer page Input Validation Whitelist Protection to Recommendations 
+-feature#6063: Change installer page Input Validation Whitelist Protection to Recommendations
 -feature#6066: Enable Drag and Drop for External Links
 -feature: Allow messages to be popup notifications
 -feature: Upgrade billboard.js to version 3.14.1
@@ -190,6 +190,12 @@ Cacti CHANGELOG
 -feature: Remove legacy Lotus Notes Attachment Types from Reports
 
 1.2.29
+-security#GHSA-pv2c-97pp-vxwg: Local File Inclusion (LFI) Vulnerability via Poller Standard Error Log Path
+-security#GHSA-f9c7-7rc3-574c: SQL Injection vulnerability when using tree rules through Automation API
+-security#GHSA-vj9g-P7F2-4wqj: SQL Injection vulnerability when view host template
+-security#GHSA-fh3x-69rr-qqpp: SQL Injection vulnerability when request automation devices
+-security#GHSA-c5j8-jxj3-hh36: Authenticated RCE via multi-line SNMP responses
+-security#GHSA-fxrq-fr7h-9rqq: Arbitrary File Creation leading to RCE
 -issue#5843: Issue with temporary tables with use of microtime
 -issue#5847: Presets Time in Cacti 1.2.28 Not Automatically Updating...
 -issue#5848: RRDfile Auto Clean not working
@@ -204,7 +210,7 @@ Cacti CHANGELOG
 -issue#5961: Issues with Replication of Tables and Misleading Full Sync Time
 -issue#5963: Cacti caching for db_table_exists() and db_column_exists() is not connection aware and replication does not replicate empty table structures
 -issue#5986: PHP ERRORS with php8.4 while including global_arrays.php
--issue#5987: Please add a new line to upgrade_database script 
+-issue#5987: Please add a new line to upgrade_database script
 -feature#5921: Add HPE Nimble/Alletra template
 -feature#5933: Only attempt to change the table type and character encoding for built-in Cacti tables
 

automation_devices.php

@@ -474,14 +474,14 @@ function get_discovery_results(&$total_rows = 0, $rows = 0, $export = false) {
 		$sql_params[] = $snmp;
 	}
 
-	if ($os != '-1') {
-		$sql_where   .= ($sql_where != '' ? ' AND ':'WHERE ') . 'os= ?';
+	if ($os != '-1' && in_array($os, $os_arr)) {
+		$sql_where   .= ($sql_where != '' ? ' AND ':'WHERE ') . 'os = ?';
 		$sql_params[] = $network;
 	}
 
 	if ($filter != '') {
 		$sql_where .= ($sql_where != '' ? ' AND ':'WHERE ') .
-			'(hostname LIKE ? OR ip LIKE ? OR sysName LIKE ? OR sysDescr ? OR sysLocation ? OR sysContact LIKE ?)';
+			'(hostname LIKE ? OR ip LIKE ? OR sysName LIKE ? OR sysDescr LIKE ? OR sysLocation LIKE ? OR sysContact LIKE ?)';
 
 		$sql_params[] = "%$filter%";
 		$sql_params[] = "%$filter%";
@@ -563,7 +563,7 @@ function create_filter() {
 				'os' => array(
 					'method'        => 'drop_array',
 					'friendly_name' => __('OS'),
-					'filter'        => FILTER_VALIDATE_INT,
+					'filter'        => FILTER_DEFAULT,
 					'default'       => '-1',
 					'pageset'       => true,
 					'array'         => $os_arr,

automation_graph_rules.php

@@ -293,6 +293,19 @@ function form_save() {
 		$save['operator']  = form_input_validate((isset_request_var('operator') ? get_nfilter_request_var('operator') : ''), 'operator', '^[0-9]+$', true, 3);
 		$save['pattern']   = form_input_validate((isset_request_var('pattern') ? get_nfilter_request_var('pattern') : ''), 'pattern', '', true, 3);
 
+		/* Test for SQL injections */
+		$field_name = str_replace(array('ht.', 'h.', 'gt.'), '', $save['field']);
+
+		if (!db_column_exists('host', $field_name) && !db_column_exists('host_template', $field_name) && !db_column_exists('graph_templates', $field_name)) {
+			raise_messsage('sql_injection', __('An attempt was made to perform a SQL injection in Tree automation'), MESSAGE_LEVEL_ERROR);
+
+			cacti_log(sprintf('ERROR: An attempt was made to perform a SQL Injection in Graph Automation from client address \'%s\'', get_client_addr()), false, 'SECURITY');
+
+			header('Location: automation_graph_rules.php?header=false&action=edit&id=' . get_request_var('id') . '&rule_type=' . AUTOMATION_RULE_TYPE_GRAPH_ACTION);
+
+			exit;
+		}
+
 		if (!is_error_message()) {
 			$item_id = sql_save($save, 'automation_graph_rule_items');
 

automation_tree_rules.php

@@ -310,6 +310,19 @@ function form_save() {
 		$save['operator']  = form_input_validate((isset_request_var('operator') ? get_nfilter_request_var('operator') : ''), 'operator', '^[0-9]+$', true, 3);
 		$save['pattern']   = form_input_validate((isset_request_var('pattern') ? get_nfilter_request_var('pattern') : ''), 'pattern', '', true, 3);
 
+		/* Test for SQL injections */
+		$field_name = str_replace(array('ht.', 'h.', 'gt.'), '', $save['field']);
+
+		if (!db_column_exists('host', $field_name) && !db_column_exists('host_template', $field_name) && !db_column_exists('graph_templates', $field_name)) {
+			raise_messsage('sql_injection', __('An attempt was made to perform a SQL injection in Tree automation'), MESSAGE_LEVEL_ERROR);
+
+			cacti_log(sprintf('ERROR: An attempt was made to perform a SQL Injection in Tree automation from client address \'%s\'', get_client_addr()), false, 'SECURITY');
+
+			header('Location: automation_tree_rules.php?header=false&action=item_edit&id=' . get_request_var('id') . '&item_id=' . (empty($item_id) ? get_request_var('item_id') : $item_id) . '&rule_type=' . AUTOMATION_RULE_TYPE_TREE_MATCH);
+
+			exit;
+		}
+
 		if (!is_error_message()) {
 			$item_id = sql_save($save, 'automation_match_rule_items');
 

lib/functions.php

@@ -5083,9 +5083,14 @@ function cacti_escapeshellarg(string $string, bool $quote = true): string {
 		return $string;
 	}
 
-	/* we must use an apostrophe to escape community names under Unix in case the user uses
-	characters that the shell might interpret. the ucd-snmp binaries on Windows flip out when
-	you do this, but are perfectly happy with a quotation mark. */
+	/* remove any carriage returns or line feeds from the argument */
+	$string = str_replace(array("\n", "\r"), array('', ''), $string);
+
+	/*
+	 * we must use an apostrophe to escape community names under Unix in case the user uses
+	 * characters that the shell might interpret. the ucd-snmp binaries on Windows flip out when
+	 * you do this, but are perfectly happy with a quotation mark.
+	 */
 	if ($config['cacti_server_os'] == 'unix') {
 		$string = escapeshellarg($string);
 
@@ -5096,12 +5101,14 @@ function cacti_escapeshellarg(string $string, bool $quote = true): string {
 			return substr($string, 1, (strlen($string) - 2));
 		}
 	} else {
-		/* escapeshellarg takes care of different quotation for both linux and windows,
+		/**
+		 * escapeshellarg takes care of different quotation for both linux and windows,
 		 * but unfortunately, it blanks out percent signs
 		 * we want to keep them, e.g. for GPRINT format strings
 		 * so we need to create our own escapeshellarg
 		 * on windows, command injection requires to close any open quotation first
-		 * so we have to escape any quotation here */
+		 * so we have to escape any quotation here
+		 */
 		if (substr_count($string, CACTI_ESCAPE_CHARACTER)) {
 			$string = str_replace(CACTI_ESCAPE_CHARACTER, '\\' . CACTI_ESCAPE_CHARACTER, $string);
 		}

lib/html_validate.php

@@ -84,7 +84,7 @@ function html_log_input_error($variable) {
  * @param string|null $variable The name of the variable that caused the validation error.
  * @param string|null $value The value of the variable that caused the validation error.
  * @param string $message An optional custom error message.
- * 
+ *
  * @return void
  */
 function die_html_input_error($variable = null, $value = null, $message = '') {

lib/snmp.php

@@ -489,7 +489,7 @@ function cacti_snmp_session_walk($session, $oid, $dummy = false, $max_repetition
 	}
 
 	if (cacti_sizeof($out)) {
-		foreach($out as $oid => $value){
+		foreach($out as $oid => $value) {
 			if (is_array($value)) {
 				foreach($value as $index => $sval) {
 					$out[$oid][$index] = format_snmp_string($sval, false, $value_output_format);
@@ -593,6 +593,18 @@ function cacti_snmp_session_getnext($session, $oid) {
 	return $out;
 }
 
+function cacti_snmp_validate_oid($oid) {
+	$oid = ltrim($oid, '.');
+
+	$validate = array_unique(array_map('is_numeric', explode('.', $oid)));
+
+	if (array_search(false, $validate, true)) {
+		return false;
+	} else {
+		return true;
+	}
+}
+
 function cacti_snmp_walk($hostname, $community, $oid, $version, $auth_user = '', $auth_pass = '',
 	$auth_proto = '', $priv_pass = '', $priv_proto = '', $context = '',
 	$port = 161, $timeout_ms = 500, $retries = 0, $bulk_walk_size = 10, $environ = 'SNMP',
@@ -737,9 +749,17 @@ function cacti_snmp_walk($hostname, $community, $oid, $version, $auth_user = '',
 
 			foreach ($temp_array as $index => $value) {
 				if (preg_match('/(.*) =.*/', $value)) {
-					$parts = explode('=', $value, 2);
-					$snmp_array[$i]['oid']   = trim($parts[0]);
-					$snmp_array[$i]['value'] = format_snmp_string($parts[1], false, $value_output_format);
+					$parts   = explode('=', $value, 2);
+					$t_oid   = trim($parts[0]);
+					$t_value = $parts[1];
+
+					if (!cacti_snmp_validate_oid($t_oid)) {
+						cacti_log(sprintf('WARNING: SNMP Agent exploit attempted on SNMP agent from host ip: %s with oid: %s', $hostname, $t_oid), false, 'SECURITY');
+						continue;
+					}
+
+					$snmp_array[$i]['oid']   = $t_oid;
+					$snmp_array[$i]['value'] = $t_value;
 					$i++;
 				} else {
 					$snmp_array[$i - 1]['value'] .= $value;
@@ -748,6 +768,15 @@ function cacti_snmp_walk($hostname, $community, $oid, $version, $auth_user = '',
 		}
 	}
 
+	/**
+	 * replay the array to escape value data in case of a multi-line exploit
+	 */
+	if (cacti_sizeof($snmp_array)) {
+		foreach($snmp_array as $index => $data) {
+			$snmp_array[$index]['value'] = format_snmp_string($data['value'], false, $value_output_format);
+		}
+	}
+
 	return $snmp_array;
 }
 

scripts/ss_net_snmp_disk_bytes.php

@@ -260,8 +260,8 @@ function ss_net_snmp_disk_bytes($host_id_or_hostname = '') {
 		}
 
 		if (!db_table_exists('host_value_cache')) {
-			$data = "'" . json_encode($current) . "'";
-			shell_exec("echo $data > $tmpdir/$tmpfile");
+			$data = json_encode($current);
+			file_put_contents("$tmpdir/$tmpfile", $data);
 		} else {
 			$data = json_encode($current);
 

scripts/ss_net_snmp_disk_io.php

@@ -263,9 +263,8 @@ function ss_net_snmp_disk_io($host_id_or_hostname = '') {
 
 
 		if (!db_table_exists('host_value_cache')) {
-			$data = "'" . json_encode($current) . "'";
-
-			shell_exec("echo $data > $tmpdir/$tmpfile");
+			$data = json_encode($current);
+			file_put_contents("$tmpdir/$tmpfile", $data);
 		} else {
 			$data = json_encode($current);