hardening: migrate quicktree SQL helpers to prepared variants

composer.json

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

quicktree.php

@@ -27,6 +27,7 @@
 
 chdir('../../');
 include_once('include/auth.php');
+include_once('plugins/quicktree/quicktree_security.php');
 
 define('QUICKTREE_BASE_URI', $config['url_path'] . 'plugins/quicktree/');
 
@@ -45,6 +46,7 @@
 set_default_action();
 
 $action = get_request_var('action');
+$location = quicktree_normalize_location(get_nfilter_request_var('location'));
 $user   = $_SESSION['sess_user_id'];
 
 /* ================= input validation ================= */
@@ -55,8 +57,6 @@
 	$action = $code_actions[$drp_action];
 }
 
-header('action_3_new: '. $action);
-
 switch ($action) {
 	case 'add':
 		$graph = 0;
@@ -152,9 +152,10 @@
 
 		html_start_box($form_actions[$drp_action], '60%', '', '3', 'center', '');
 
-		$queryrows = db_fetch_assoc("SELECT g.id, g.name
+		$queryrows = db_fetch_assoc_prepared('SELECT g.id, g.name
 			FROM graph_tree AS g
-			ORDER BY g.name");
+			ORDER BY g.name',
+			array());
 
 		print '<tr><td>';
 
@@ -205,7 +206,7 @@
 	case 'clear':
 		$SQL = db_execute_prepared('DELETE FROM quicktree_graphs WHERE userid = ?', array($user));
 
-		header('Location: quicktree.php?header=false&drp_action=&action=&location=' . get_nfilter_request_var('location'));
+		header('Location: quicktree.php?header=false&drp_action=&action=&location=' . $location);
 
 		break;
 	case 'save':
@@ -252,7 +253,9 @@
 			include_once($config['base_path'] . '/lib/api_tree.php');
 
 			if (empty($new_tree_id)) {
-				$seq = db_fetch_cell('SELECT MAX(sequence) FROM graph_tree');
+				$seq = db_fetch_cell_prepared('SELECT MAX(sequence)
+					FROM graph_tree',
+					array());
 
 				if ($seq == NULL || $seq < 0) {
 					$seq = 1;
@@ -333,7 +336,7 @@
 				array($user, $graph));
 		}
 
-		header('Location: quicktree.php?location=' . get_nfilter_request_var('location'));
+		header('Location: quicktree.php?location=' . $location);
 
 		break;
 	case 'add_ajax':
@@ -343,13 +346,13 @@
 
 		break;
 	default:
-		if (get_nfilter_request_var('location') == 'console') {
+		if ($location == 'console') {
 			top_header();
 		} else {
 			general_header();
 		}
 
-		form_start('quicktree.php?location=' . get_nfilter_request_var('location'), 'quicktree_form');
+		form_start('quicktree.php?location=' . $location, 'quicktree_form');
 		html_start_box(__('QuickTree', 'quicktree'), '100%', true, '3', 'center', '');
 
 		print "<div class='spacer formHeader collapsible' id='row_info'>
@@ -411,7 +414,7 @@
 
 				print '<table class="cactiTable"><thead><tr><th class="center">' . $graph_title;
 
-				print '&nbsp;&nbsp;<a class="pic iconLink" href="' . html_escape('quicktree.php?location=' . get_nfilter_request_var('location') . '&action=remove&id=' . $gr['id'])
+				print '&nbsp;&nbsp;<a class="pic iconLink" href="' . html_escape('quicktree.php?location=' . $location . '&action=remove&id=' . $gr['id'])
 					. '" title="' . __esc('Remove This Graph From QuickTree', 'quicktree') . '"><i class="deviceDown fas fa-times-circle"></i></a>';
 				print '</th></tr></thead>';
 
@@ -430,4 +433,3 @@
 
 		break;
 }
-

quicktree_security.php

@@ -0,0 +1,17 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2010-2022 Howard Jones                                    |
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+function quicktree_normalize_location($value) {
+	if ($value === 'console') {
+		return 'console';
+	}
+
+	return 'tab';
+}

setup.php

@@ -237,9 +237,10 @@ function quicktree_check_upgrade() {
     $info    = plugin_quicktree_version();
     $current = $info['version'];
 
-	$old = db_fetch_cell('SELECT version
+	$old = db_fetch_cell_prepared('SELECT version
 		FROM plugin_config
-		WHERE directory = "quicktree"');
+		WHERE directory = ?',
+		array('quicktree'));
 
     if ($current != $old) {
     	api_plugin_register_hook('quicktree', 'page_head', 'quicktree_page_head', 'setup.php', 1);
@@ -263,4 +264,3 @@ function quicktree_check_upgrade() {
 		}
     }
 }
-

tests/E2E/QuicktreeNoDebugHeaderTest.php

@@ -0,0 +1,20 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+describe('quicktree security regression wiring', function () {
+	it('does not emit the raw debug action header anymore', function () {
+		$path = realpath(__DIR__ . '/../../quicktree.php');
+		expect($path)->not->toBeFalse();
+
+		$contents = file_get_contents($path);
+		expect($contents)->not->toBeFalse();
+
+		expect($contents)->not->toContain("header('action_3_new: '. \$action);");
+	});
+});

tests/Integration/QuicktreeRedirectSafetyTest.php

@@ -0,0 +1,23 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+describe('quicktree redirect and url wiring', function () {
+	it('normalizes location before redirects and form links', function () {
+		$path = realpath(__DIR__ . '/../../quicktree.php');
+		expect($path)->not->toBeFalse();
+
+		$contents = file_get_contents($path);
+		expect($contents)->not->toBeFalse();
+
+		expect($contents)->toContain("include_once('plugins/quicktree/quicktree_security.php');");
+		expect($contents)->toContain("\$location = quicktree_normalize_location(get_nfilter_request_var('location'));");
+		expect($contents)->toContain("header('Location: quicktree.php?location=' . \$location);");
+		expect($contents)->toContain("form_start('quicktree.php?location=' . \$location, 'quicktree_form');");
+	});
+});

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