From 0058fef4828e26ce2284bdc8f7dac75b0180df09 Mon Sep 17 00:00:00 2001 From: TheWitness Date: Sun, 26 Jan 2025 12:53:31 -0500 Subject: [PATCH] QA: Merging Security Updates from 1.2.x into develop --- CHANGELOG | 14 +++++++---- automation_devices.php | 8 +++---- automation_graph_rules.php | 13 +++++++++++ automation_tree_rules.php | 13 +++++++++++ lib/functions.php | 17 ++++++++++---- lib/html_validate.php | 2 +- lib/snmp.php | 37 ++++++++++++++++++++++++++---- scripts/ss_net_snmp_disk_bytes.php | 4 ++-- scripts/ss_net_snmp_disk_io.php | 5 ++-- 9 files changed, 90 insertions(+), 23 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 94f9c74abd..18a0411634 100644 --- a/CHANGELOG +++ b/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 diff --git a/automation_devices.php b/automation_devices.php index 9835024f04..7c4f9126a4 100644 --- a/automation_devices.php +++ b/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, diff --git a/automation_graph_rules.php b/automation_graph_rules.php index 07aeb31925..91646e885c 100644 --- a/automation_graph_rules.php +++ b/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'); diff --git a/automation_tree_rules.php b/automation_tree_rules.php index f82c389463..343203fd31 100644 --- a/automation_tree_rules.php +++ b/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'); diff --git a/lib/functions.php b/lib/functions.php index 03bc10ec28..cfcf3bff03 100644 --- a/lib/functions.php +++ b/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); } diff --git a/lib/html_validate.php b/lib/html_validate.php index 58293c68b7..6faf009e4b 100644 --- a/lib/html_validate.php +++ b/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 = '') { diff --git a/lib/snmp.php b/lib/snmp.php index fb196846c9..d393de8f6c 100644 --- a/lib/snmp.php +++ b/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; } diff --git a/scripts/ss_net_snmp_disk_bytes.php b/scripts/ss_net_snmp_disk_bytes.php index 954d0e2d0f..4695aefd9a 100644 --- a/scripts/ss_net_snmp_disk_bytes.php +++ b/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); diff --git a/scripts/ss_net_snmp_disk_io.php b/scripts/ss_net_snmp_disk_io.php index 54cc42a1c5..d39c4f6fcf 100644 --- a/scripts/ss_net_snmp_disk_io.php +++ b/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);