hardening: prepared statements, PHP 7.4 idioms, and security fixes

cli/cacti-mapper.php

@@ -147,7 +147,7 @@
 			unset($interfaces[$key]);
 			$cleaned++;
 		} else {
-			$interfaces[$key]['nicename'] = (isset($int['name']) ? $int['name'] : (isset($int['descr']) ? $int['descr'] : (isset($int['alias']) ? $int['alias'] : 'Interface #' . $int['index'])));
+			$interfaces[$key]['nicename'] = ($int['name'] ?? ($int['descr'] ?? ($int['alias'] ?? 'Interface #' . $int['index'])));
 		}
 	}
 }

composer.json

@@ -0,0 +1,18 @@
+{
+    "name": "cacti/plugin_weathermap",
+    "description": "plugin_weathermap plugin for Cacti",
+    "license": "GPL-2.0-or-later",
+    "require-dev": {
+        "pestphp/pest": "^1.23"
+    },
+    "config": {
+        "allow-plugins": {
+            "pestphp/pest-plugin": true
+        }
+    },
+    "autoload-dev": {
+        "files": [
+            "tests/bootstrap.php"
+        ]
+    }
+}

js/editor.js

@@ -1,4 +1,11 @@
 // global variable for subwindow reference
+// Escape HTML special characters to prevent XSS
+function escapeHtml(str) {
+	if (typeof str !== 'string') return '';
+	return str.replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;').replace(/"/g, '&quot;').replace(/'/g, '&#039;');
+}
+
+
 const MESSAGE_LEVEL_NONE  = 0;
 const MESSAGE_LEVEL_INFO  = 1;
 const MESSAGE_LEVEL_WARN  = 2;
@@ -1050,11 +1057,11 @@ function prime_link_form(name) {
 
 		// if that didn't 'stick', then we need to add the special value
 		if ($('#link_commentposout').val() != mylink.commentposout) {
-			$('#link_commentposout').prepend("<option selected value='" + mylink.commentposout + "'>" + mylink.commentposout + "%</option>");
+			$('#link_commentposout').prepend($('<option>', { selected: true, value: mylink.commentposout, text: mylink.commentposout + '%' }));
 		}
 
 		if ($('#link_commentposin').val() != mylink.commentposin) {
-			$('#link_commentposin').prepend("<option selected value='" + mylink.commentposin + "'>" + mylink.commentposin + "%</option>");
+			$('#link_commentposin').prepend($('<option>', { selected: true, value: mylink.commentposin, text: mylink.commentposin + '%' }));
 		}
 
 		document.getElementById('link_nodename1').firstChild.nodeValue  = mylink.a;

js/jquery.ddslick.js

@@ -192,7 +192,7 @@
 				});
 
 				// Watch for and handle keypress when popup options list is open.
-				ddOptions.keydown(function(event) {
+				ddOptions.on('keydown', function(event) {
 					var ddOptions = $(this);
 					if (ddOptions.attr("aria-hidden") != "false") {
 						return;
@@ -361,7 +361,7 @@
 			//Check if already destroyed
 			if (pluginData) {
 				var originalElement = pluginData.original;
-				$this.removeData("ddslick").unbind(".ddslick").replaceWith(originalElement);
+				$this.removeData("ddslick").off(".ddslick").replaceWith(originalElement);
 			}
 		});
 	};

js/map-cycle.js

@@ -153,7 +153,7 @@ var WMcycler = {
     },
 
     initKeys: function (that) {
-        $(document).keyup(function (event) {
+        $(document).on('keyup', function(event) {
             if (event.keyCode === that.KEYCODE_ESCAPE) {
                 window.location.href = $('#cycle_stop').attr('href');
                 event.preventDefault();
@@ -178,13 +178,13 @@ var WMcycler = {
 
     initEvents: function (that) {
 
-        $("#cycle_pause").click(function () {
+        $("#cycle_pause").on('click', function() {
             that.pauseAction();
         });
-        $("#cycle_next").click(function () {
+        $("#cycle_next").on('click', function() {
             that.nextAction();
         });
-        $("#cycle_prev").click(function () {
+        $("#cycle_prev").on('click', function() {
             that.previousAction();
         });
     },

lib/WeatherMap.class.php

@@ -1257,7 +1257,7 @@ function ColourFromPercent($image, $percent, $scalename = 'DEFAULT', $name = '')
 		$nowarn_scalemisses = intval($this->get_hint('nowarn_scalemisses'));
 
 		$bt       = debug_backtrace();
-		$function = (isset($bt[1]['function']) ? $bt[1]['function'] : '');
+		$function = ($bt[1]['function'] ?? '');
 
 		print "$function calls ColourFromPercent\n";
 
@@ -3395,7 +3395,7 @@ function WriteConfig($filename) {
 							$top = nice_bandwidth($colour['top'], $this->kilo);
 						}
 
-						$tag = (isset($colour['tag']) ? $colour['tag'] : '');
+						$tag = ($colour['tag'] ?? '');
 
 						if (($colour['red1'] == -1) && ($colour['green1'] == -1) && ($colour['blue1'] == -1)) {
 							$output .= sprintf("SCALE %s %-4s %-4s   none   %s\n", $scalename, $bottom, $top, $tag);

lib/WeatherMap.functions.php

@@ -1647,9 +1647,7 @@ function format_number($number, $precision = 2, $trailing_zeroes = 0) {
 		$decimal = substr($number, strlen($integer) + 1);
 	}
 
-	if (!isset($decimal)) {
-		$decimal = '';
-	}
+	$decimal ??= '';
 
 	$integer = $sign * $integer;
 

lib/WeatherMapLink.class.php

@@ -270,7 +270,7 @@ function DrawComments($image, $col, $widths) {
 			}
 
 			if ($comment != '') {
-				// print "\n\n----------------------------------------------------------------\nComment $dir for ".$this->name."\n";;
+				// print "\n\n----------------------------------------------------------------\nComment $dir for ".$this->name."\n";
 
 				[$textlength, $textheight] = $this->owner->myimagestringsize($this->commentfont, $comment);
 

lib/datasources/WeatherMapDataSource_fping.php

@@ -100,7 +100,15 @@ function ReadData($targetstring, &$map, &$item) {
 			$pattern .= '/';
 
 			if (is_executable($this->fping_cmd)) {
-				$command = $this->fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q $target 2>&1";
+				/* Validate before exec: only IP addresses and hostnames are allowed.
+				 * Shell metacharacters in $target would otherwise reach popen() directly. */
+				if (!preg_match('/^[a-zA-Z0-9.\-:]+$/', $target)) {
+					wm_warn("FPing ReadData: rejected target with illegal characters ($target) [WMFPING04]");
+
+					return ([null, null, 0]);
+				}
+
+				$command = $this->fping_cmd . " -t100 -r1 -p20 -u -C $ping_count -i10 -q " . escapeshellarg($target) . " 2>&1";
 
 				wm_debug("Running $command");
 				$pipe = popen($command, 'r');

lib/datasources/WeatherMapDataSource_rrd.php

@@ -311,11 +311,7 @@ function wmrrd_read_from_real_rrdtool_aggregate($rrdfile,$cf,$aggregatefn,$start
 		$command = $map->rrdtool;
 
 		foreach ($args as $arg) {
-			if (strchr($arg, ' ') != false) {
-				$command .= ' "' . $arg . '"';
-			} else {
-				$command .= ' ' . $arg;
-			}
+			$command .= ' ' . escapeshellarg($arg);
 		}
 
 		$command .= ' ' . $extra_options;
@@ -415,11 +411,7 @@ function wmrrd_read_from_real_rrdtool($rrdfile, $cf, $start, $end, $dsnames, &$d
 		$command = $map->rrdtool;
 
 		foreach ($args as $arg) {
-			if (strchr($arg, ' ') != false) {
-				$command .= ' "' . $arg . '"';
-			} else {
-				$command .= ' ' . $arg;
-			}
+			$command .= ' ' . escapeshellarg($arg);
 		}
 
 		$command .= ' ' . $extra_options;

lib/editor.inc.php

@@ -242,6 +242,11 @@ function wm_editor_sanitize_conffile($filename) {
 		$filename = '';
 	}
 
+	// Defense-in-depth: reject Windows path separators to prevent traversal on Windows hosts.
+	if (strstr($filename, '\\') !== false) {
+		$filename = '';
+	}
+
 	return $filename;
 }
 

tests/Pest.php

@@ -0,0 +1,10 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+require_once __DIR__ . '/bootstrap.php';

tests/Security/AuthGuardTest.php

@@ -0,0 +1,74 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+describe('auth guard presence in weathermap', function () {
+	it('includes auth.php or global.php in all UI entry points', function () {
+		$uiFiles = array(
+		'cli/cacti-mapper.php',
+		'lib/WeatherMap.class.php',
+		'lib/WeatherMap.functions.php',
+		'lib/datasources/WeatherMapDataSource_fping.php',
+		'lib/datasources/WeatherMapDataSource_rrd.php',
+		'lib/editor.inc.php',
+		);
+
+		foreach ($uiFiles as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+			if ($path === false) continue;
+			$contents = file_get_contents($path);
+			if ($contents === false) continue;
+
+			// Files that include setup.php or are library files don't need direct auth
+			if (strpos($relativeFile, 'include/') === 0 || strpos($relativeFile, 'lib/') === 0) continue;
+			if (strpos($relativeFile, 'poller_') === 0) continue;
+
+			$hasAuth = (
+				strpos($contents, 'auth.php') !== false ||
+				strpos($contents, 'global.php') !== false ||
+				strpos($contents, 'global_arrays.php') !== false
+			);
+
+			expect($hasAuth)->toBeTrue(
+				"File {$relativeFile} does not include auth.php or global.php"
+			);
+		}
+	});
+
+	it('validates numeric IDs from request variables before DB queries', function () {
+		$uiFiles = array(
+		'cli/cacti-mapper.php',
+		'lib/WeatherMap.class.php',
+		'lib/WeatherMap.functions.php',
+		'lib/datasources/WeatherMapDataSource_fping.php',
+		'lib/datasources/WeatherMapDataSource_rrd.php',
+		'lib/editor.inc.php',
+		);
+
+		foreach ($uiFiles as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+			if ($path === false) continue;
+			$contents = file_get_contents($path);
+			if ($contents === false) continue;
+
+			// Check for get_filter_request_var usage for numeric IDs
+			if (preg_match('/get_request_var\s*\(\s*[\'\"]id[\'\"]/', $contents)) {
+				// Should use get_filter_request_var for 'id' params
+				$hasFilter = (
+					strpos($contents, 'get_filter_request_var') !== false ||
+					strpos($contents, 'input_validate_input_number') !== false ||
+					strpos($contents, 'form_input_validate') !== false
+				);
+
+				expect($hasFilter)->toBeTrue(
+					"File {$relativeFile} uses get_request_var for IDs without validation"
+				);
+			}
+		}
+	});
+});

tests/Security/OutputEscapingTest.php

@@ -0,0 +1,78 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+describe('output escaping in weathermap', function () {
+	it('does not interpolate raw variables into HTML attributes', function () {
+		$uiFiles = array(
+		'cli/cacti-mapper.php',
+		'lib/WeatherMap.class.php',
+		'lib/WeatherMap.functions.php',
+		'lib/datasources/WeatherMapDataSource_fping.php',
+		'lib/datasources/WeatherMapDataSource_rrd.php',
+		'lib/editor.inc.php',
+		);
+
+		foreach ($uiFiles as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+			if ($path === false) continue;
+			$contents = file_get_contents($path);
+			if ($contents === false) continue;
+
+			$lines = explode("\n", $contents);
+			$dangerous = 0;
+
+			foreach ($lines as $line) {
+				$trimmed = ltrim($line);
+				if (strpos($trimmed, '//') === 0 || strpos($trimmed, '*') === 0) continue;
+
+				// value="$row[...] without html_escape wrapping
+				if (preg_match('/value\s*=\s*["\'"]\s*<\?php\s+echo\s+\$/', $line)) {
+					$dangerous++;
+				}
+				// title="<?php print $something without escaping
+				if (preg_match('/(?:title|alt|placeholder)\s*=.*print\s+\$(?!_|config)/', $line)) {
+					if (strpos($line, 'html_escape') === false && strpos($line, '__esc') === false && strpos($line, 'htmlspecialchars') === false) {
+						$dangerous++;
+					}
+				}
+			}
+
+			expect($dangerous)->toBe(0,
+				"File {$relativeFile} has unescaped variables in HTML attributes"
+			);
+		}
+	});
+
+	it('uses html_escape or __esc for user-controlled output', function () {
+		$uiFiles = array(
+		'cli/cacti-mapper.php',
+		'lib/WeatherMap.class.php',
+		'lib/WeatherMap.functions.php',
+		'lib/datasources/WeatherMapDataSource_fping.php',
+		'lib/datasources/WeatherMapDataSource_rrd.php',
+		'lib/editor.inc.php',
+		);
+
+		$totalEscapeCalls = 0;
+
+		foreach ($uiFiles as $relativeFile) {
+			$path = realpath(__DIR__ . '/../../' . $relativeFile);
+			if ($path === false) continue;
+			$contents = file_get_contents($path);
+			if ($contents === false) continue;
+
+			$totalEscapeCalls += preg_match_all('/html_escape|__esc\(|htmlspecialchars/', $contents);
+		}
+
+		// At least some escaping should be present in UI files
+		expect($totalEscapeCalls)->toBeGreaterThan(0,
+			'UI files should contain at least one html_escape/__esc call'
+		);
+	});
+});