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

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

@@ -86,7 +86,7 @@ jobs:
       run: sudo apt-get update
 
     - name: Install System Dependencies
-      run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping libapache2-mod-php${{ matrix.php }}
+      run: sudo apt-get install -y apache2 snmp snmpd rrdtool fping
 
     - name: Start SNMPD Agent and Test
       run: |

composer.json

@@ -0,0 +1,18 @@
+{
+    "name": "cacti/plugin_maint",
+    "description": "plugin_maint 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"
+        ]
+    }
+}

functions.php

@@ -117,6 +117,11 @@ function plugin_maint_check_schedule(int $schedule): bool {
 			case 2: // Recurring
 				// past, calculate next
 				if ($sc['etime'] < $t) {
+					// minterval=0 would produce a zero-duration DateInterval and loop forever (FIND-004)
+					if ($sc['minterval'] <= 0) {
+						return false;
+					}
+
 					// convert start and end to local so that hour stays same for add days across daylight saving time change
 					$starttimelocal = (new DateTime('@' . strval($sc['stime'])))->setTimezone(new DateTimeZone(date_default_timezone_get()));
 					$endtimelocal   = (new DateTime('@' . strval($sc['etime'])))->setTimezone(new DateTimeZone(date_default_timezone_get()));

maint.php

@@ -764,7 +764,7 @@ function schedule_edit(): void {
 				'max_length'    => 100,
 				'default'       => $maint_item_data['name'],
 				'description'   => __('Provide the Maintenance Schedule a meaningful name', 'maint'),
-				'value'         => isset($maint_item_data['name']) ? $maint_item_data['name'] : '',
+				'value'         => $maint_item_data['name'] ?? '',
 			],
 			'enabled' => [
 				'friendly_name' => __('Enabled', 'maint'),
@@ -779,15 +779,15 @@ function schedule_edit(): void {
 				'on_change'     => 'changemaintType()',
 				'array'         => $maint_types,
 				'description'   => __('The type of schedule, one time or recurring.', 'maint'),
-				'value'         => isset($maint_item_data['mtype']) ? $maint_item_data['mtype'] : '',
+				'value'         => $maint_item_data['mtype'] ?? '',
 			],
 			'minterval' => [
 				'friendly_name' => __('Interval', 'maint'),
 				'method'        => 'drop_array',
 				'array'         => $maint_intervals,
 				'default'       => 86400,
 				'description'   => __('This is the interval in which the start / end time will repeat.', 'maint'),
-				'value'         => isset($maint_item_data['minterval']) ? $maint_item_data['minterval'] : '1',
+				'value'         => $maint_item_data['minterval'] ?? '1',
 			],
 			'stime' => [
 				'friendly_name' => __('Start Time', 'maint'),
@@ -975,7 +975,7 @@ function schedules(): void {
 
 			form_selectable_cell($maint_intervals[$schedule['minterval']], $schedule['id']);
 			form_selectable_cell($yesno[$schedule['enabled']], $schedule['id']);
-			form_checkbox_cell($schedule['name'], $schedule['id']);
+			form_checkbox_cell(html_escape($schedule['name']), $schedule['id']);
 			form_end_row();
 		}
 	} else {

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,66 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+describe('auth guard presence in maint', function () {
+	it('includes auth.php or global.php in all UI entry points', function () {
+		$uiFiles = array(
+		'functions.php',
+		'maint.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(
+		'functions.php',
+		'maint.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,86 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+describe('output escaping in maint', function () {
+	it('does not interpolate raw variables into HTML attributes', function () {
+		$uiFiles = [
+		'functions.php',
+		'maint.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 = [
+		'functions.php',
+		'maint.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'
+		);
+	});
+});