Resolving Issue #2662

* HRULE objects broken in some cases * Also two missed escapeshell* change
Cacti · May 12, 2019 · d0c3ab0 · d0c3ab0
1 parent 4b1adc1
commit d0c3ab0
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 17 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -27,6 +27,7 @@ Cacti CHANGELOG
 -issue#2649: Automation not creating graphs when there are custom items
 -issue#2650: Some PHP Notice like "undefined variables"
 -issue#2652: Honor php.ini when php is used in exec() and shell_exec() calls
+-issue#2662: HRULE objects broken in some cases
 -issue#2668: RFC1213 sysDescr trailing parentheses lost
 -issue#2672: Cacti Install on Windows Fails
 -issue#2676: Skin paper plane not working on iPhone XR

diff --git a/lib/functions.php b/lib/functions.php
@@ -1473,26 +1473,39 @@ function prepare_validate_result(&$result) {
 			return ($space_cnt+1 == $delim_cnt);
 		}
 	} else {
-		/* strip all non numeric data */
-		$result = preg_replace('/[^0-9,.+-]/', '', $result);
+		$result = strip_alpha($result);
 
-		/* check the easy cases first */
-		/* it has no delimiters, and no space, therefore, must be numeric */
-		if (is_numeric($result)) {
-			return true;
-		} elseif (is_float($result)) {
-			return true;
-		} else {
+		if ($result === false) {
 			$result = 'U';
 			return false;
+		} else {
+			return true;
 		}
 	}
 }
 
-/* get_full_script_path - gets the full path to the script to execute to obtain data for a
-     given data source. this function does not work on SNMP actions, only script-based actions
-   @arg $local_data_id - (int) the ID of the data source
-   @returns - the full script path or (bool) false for an error */
+/** strip_alpha - remove non-numeric data from a string and return the numeric part
+ *  @arg $string - (char) the string to be evaluated
+ *  @returns - either the numeric value or false if not numeric
+*/
+function strip_alpha($string) {
+	/* strip all non numeric data */
+	$string = trim(preg_replace('/[^0-9,.+-]/', '', $string));
+
+	/* check the easy cases first */
+	/* it has no delimiters, and no space, therefore, must be numeric */
+	if (is_numeric($string) || is_float($string)) {
+		return $string;
+	} else {
+		return false;
+	}
+}
+
+/** get_full_script_path - gets the full path to the script to execute to obtain data for a
+ *    given data source. this function does not work on SNMP actions, only script-based actions
+ *  @arg $local_data_id - (int) the ID of the data source
+ *  @returns - the full script path or (bool) false for an error 
+*/
 function get_full_script_path($local_data_id) {
 	global $config;
 
@@ -3989,14 +4002,14 @@ function get_dns_from_ip ($ip, $dns, $timeout = 1000) {
 function poller_maintenance () {
 	global $config;
 
-	$command_string = trim(read_config_option('path_php_binary'));
+	$command_string = cacti_escapeshellcmd(read_config_option('path_php_binary'));
 
 	// If its not set, just assume its in the path
 	if (trim($command_string) == '') {
 		$command_string = 'php';
 	}
 
-	$extra_args = ' -q ' . $config['base_path'] . '/poller_maintenance.php';
+	$extra_args = ' -q ' . cacti_escapeshellarg($config['base_path'] . '/poller_maintenance.php');
 
 	exec_background($command_string, $extra_args);
 }

diff --git a/lib/rrd.php b/lib/rrd.php
@@ -2124,12 +2124,15 @@ function rrdtool_function_graph($local_graph_id, $rra_id, $graph_data_array, $rr
 					break;
 				case GRAPH_ITEM_TYPE_HRULE:
 					/* perform variable substitution; if this does not return a number, rrdtool will FAIL! */
-					$substitute = rrd_substitute_host_query_data($graph_variables['value'][$graph_item_id], $graph, $graph_item);
+					$substitute = strip_alpha(rrd_substitute_host_query_data($graph_variables['value'][$graph_item_id], $graph, $graph_item));
 
 					$text_format = rrdtool_escape_string(html_escape(rrd_substitute_host_query_data($graph_variables['text_format'][$graph_item_id], $graph, $graph_item)));
 
-					if (is_numeric($substitute)) {
+					/* don't break rrdtool if the strip_alpha() returns false */
+					if ($substitute !== false) {
 						$graph_variables['value'][$graph_item_id] = $substitute;
+					} else {
+						$graph_variables['value'][$graph_item_id] = '0';
 					}
 
 					$txt_graph_items .= $graph_item_types[$graph_item['graph_type_id']] . ':' . $graph_variables['value'][$graph_item_id] . $graph_item_color_code . ':' . cacti_escapeshellarg($text_format . $hardreturn[$graph_item_id]) . '' . $dash;