security: consolidate GHSA batch 11 fixes for develop (fq9x, hh7r, v99q, f5f5) (#8165)

* security: consolidate GHSA batch 11 fixes for develop (fq9x, hh7r, v99q, f5f5)

Mirrors the 1.2.x batch-11 security consolidation. Items already fixed on the
develop tip are credited on their advisories and omitted here:
- GHSA-54hx-hxc4-jq7j: TICK/SHIFT values already pass through strip_alpha().
- GHSA-hh7r CSV portion: already guarded via cacti_csv_cell(); only the
  Content-Disposition filename needed sanitizing (included below).
- GHSA-mx5c: REST scaffold already deny-by-default (404 unless api_enabled),
  enforced by tests/Unit/ApiDenyByDefaultTest.php.
- GHSA-rjvj/pjq5/fmhq/mfw8: already addressed on develop.

Fixes applied here:
- GHSA-fq9x-x3vf-3vf2 (High): add substitute_script_path() single-pass token
  substitution and refactor get_full_script_path()/get_full_test_script_path()
  to use a pre-escaped value map, closing the second-order re-substitution OS
  command injection.
- GHSA-hh7r-464j-rf39: sanitize the Content-Disposition filename in graph_xport.php.
- GHSA-v99q-57w7-mpmg: escape the page title rendered in the HTML <title>.
- GHSA-f5f5-h85v-6m3h: render user admin tooltip content as escaped text.

* security: add batch-11 consolidation regression tests for develop

Addresses PR #8165 review feedback by adding regression coverage for all four fixed sinks:

- GHSA-fq9x: behavioral single-pass proof that substitute_script_path() does not re-substitute a field value containing another field token, plus source guards that both path builders use the helper.

- GHSA-hh7r: Content-Disposition filename strips CR/LF/quote/backslash.

- GHSA-v99q: the HTML <title> is html_escape()'d.

- GHSA-f5f5: both the user_admin.php and user_group_admin.php tooltips render data-tooltip as escaped text.

* style: apply php-cs-fixer formatting to substitute_script_path (CI coding standards)

* docs: require composer run-script php-cs-fixit before creating any pull request

* docs: require PHPStan (level 8, satisfies Level 6 floor) to pass before creating any pull request

---------

Co-authored-by: Larry Adams <TheWitness@users.noreply.github.com>

.github/copilot-instructions.md

@@ -69,6 +69,10 @@
   - Use `cacti_redirect($path, $params)` for local redirects; it rejects absolute and protocol-relative destinations (checking both raw and percent-decoded forms) and terminates the request.
   - Escape output for HTML attribute contexts with `html_escape_attr()`, not `htmle()`/`html_escape()`; only the attribute helper double-encodes to stop pre-encoded entities from breaking out of the attribute.
 
+## Pull request pre-flight (MANDATORY)
+- **Before creating any pull request you MUST run `composer run-script php-cs-fixit`** to apply the repo's `php-cs-fixer` formatting rules, then commit any resulting changes. This script fixes in place (unlike the dry-run `composer phpcsfixer`), so creating a PR without running it is what trips the coding-standards job in CI. Run it from WSL (see Windows/WSL note below). Do not open the PR until the working tree is clean after the fixer runs.
+- **Every commit must also pass PHPStan static analysis.** Run `composer run-script phpstan -- --memory-limit=2G` from WSL and confirm it reports `No errors` before opening the PR. The default 128M limit crashes a parallel worker, so always pass `--memory-limit=2G` (or higher). The repo config (`.phpstan.neon`) analyses at **level 8**, which is cumulative and therefore satisfies the Level 6 floor; `tests/*` is excluded from analysis, so test-only files will not surface here but production code (e.g. `lib/`) will.
+
 ## Workflows you’ll actually use
 - **Windows/WSL**: When working in VS Code from a Windows machine, run all fixes, builds, linters, tests, and git operations inside WSL (a Linux distro) rather than native Windows. The toolchain (PHP 8.3, Composer, `php-cs-fixer`, Pest) and the repo's tab indentation / `\n` line-ending conventions are Linux-first; running them on native Windows produces spurious diffs and failures. Edit the WSL-mounted checkout (e.g. under `/mnt/c/...` or a native WSL path) and invoke `composer` scripts such as `composer php-cs-fixit` from the WSL shell.
 - Install deps: `composer install` (CI validates via `.github/workflows/syntax.yml`).

CHANGELOG

@@ -413,6 +413,10 @@ Cacti CHANGELOG
 -feature#7681: Add fail2ban support with a stable authentication-failure log line and a bundled filter
 
 1.2.32
+-security#GHSA-fq9x-x3vf-3vf2: Substitute data-input field tokens in a single pass so a composed value cannot re-inject an escaped payload into the script command line
+-security#GHSA-hh7r-464j-rf39: Sanitize the Content-Disposition filename in the graph data export
+-security#GHSA-v99q-57w7-mpmg: Escape the page title rendered in the HTML title element
+-security#GHSA-f5f5-h85v-6m3h: Render user administration tooltip content as escaped text
 -security#GHSA-p3rg-7pc3-2h86: Dummy-verify passwords for unknown users to close a login-timing enumeration side channel
 -security#GHSA-92jm-r83q-cjr8: Validate the hgd graph-template id as a number before the tree-view graph SQL filter
 -security#GHSA-4j2v-vxgp-83ch: Validate the hgd graph-template id for host-leaf Graph Template grouping before the SQL filter

graph_xport.php

@@ -122,8 +122,10 @@ if ($xport_array['meta']['step'] > 0) {
 	$xport_array['meta']['missing_rows'] = max(0, $xport_array['meta']['expected_rows'] - $xport_array['meta']['rows']);
 }
 
-// Make graph title the suggested file name
-$filename = $xport_array['meta']['title_cache'] . '.csv';
+// Make graph title the suggested file name; strip characters that could break
+// out of the quoted filename in the Content-Disposition header (CR/LF header
+// injection, quote escaping).
+$filename = str_replace(["\r", "\n", '"', '\\'], '', (string) $xport_array['meta']['title_cache']) . '.csv';
 
 header('Content-type: application/vnd.ms-excel; charset=UTF-8');
 header('Content-Transfer-Encoding: binary');

lib/functions.php

@@ -3167,6 +3167,29 @@ function test_data_source(int $data_template_id, int $host_id, int $snmp_query_i
 	return false;
 }
 
+/**
+ * substitute_script_path - performs a single-pass substitution of <field> tokens
+ * in a data-input command template using pre-escaped values.
+ *
+ * Each <name> token in the ORIGINAL template is replaced at most once from the
+ * supplied map; substituted values are never re-scanned, so a field whose value
+ * contains another field's <token> cannot splice an already-escaped payload into
+ * a neighbouring quoted region (second-order breakout, GHSA-fq9x-x3vf-3vf2).
+ *
+ * @param string $template       The command template containing <field> tokens
+ * @param array  $escaped_values Map of field name => already-escaped value
+ *
+ * @return string The template with all known tokens substituted once; unknown
+ *                tokens are left intact for the caller's trailing cleanup to strip
+ */
+function substitute_script_path(string $template, array $escaped_values) : string {
+	return preg_replace_callback('/<([A-Za-z0-9_]+)>/',
+		function (array $matches) use ($escaped_values) : string {
+			return array_key_exists($matches[1], $escaped_values) ? $escaped_values[$matches[1]] : $matches[0];
+		},
+		$template) ?? '';
+}
+
 /**
  * Gets the full path to the script to execute to obtain data for a
  * given data template for testing. this function does not work on
@@ -3207,6 +3230,8 @@ function get_full_test_script_path(int $data_template_id, int $host_id) : mixed
 
 	$host = db_fetch_row_prepared('SELECT * FROM host WHERE id = ?', [$host_id]);
 
+	$escaped_values = [];
+
 	if (cacti_sizeof($data) && is_array($host)) {
 		foreach ($data as $item) {
 			if (isset($host[$item['data_name']])) {
@@ -3220,13 +3245,22 @@ function get_full_test_script_path(int $data_template_id, int $host_id) : mixed
 				$value = cacti_escapeshellarg_cmd((string) $item['value']);
 			}
 
-			$full_path = str_replace('<' . $item['data_name'] . '>', $value, $full_path);
+			$escaped_values[$item['data_name']] = $value;
 		}
 	}
 
-	$search    = ['<path_cacti>', '<path_snmpget>', '<path_php_binary>'];
-	$replace   = [CACTI_PATH_BASE, read_config_option('path_snmpget'), read_config_option('path_php_binary')];
-	$full_path = str_replace($search, $replace, $full_path);
+	/* the well-known path tokens resolve to trusted configuration values; field
+	 * names take precedence so behaviour matches the historical field-first order */
+	$escaped_values += [
+		'path_cacti'      => CACTI_PATH_BASE,
+		'path_snmpget'    => read_config_option('path_snmpget'),
+		'path_php_binary' => read_config_option('path_php_binary'),
+	];
+
+	/* single-pass substitution over the original template prevents a field whose
+	 * value contains another field's <token> from re-injecting an escaped payload
+	 * into an already-quoted region (GHSA-fq9x-x3vf-3vf2) */
+	$full_path = substitute_script_path($full_path, $escaped_values);
 
 	/**
 	 * sometimes a certain input value will not have anything entered... null out these fields
@@ -3278,6 +3312,8 @@ function get_full_script_path(int $local_data_id) : mixed {
 
 	$full_path = $data_source['input_string'];
 
+	$escaped_values = [];
+
 	if (cacti_sizeof($data)) {
 		foreach ($data as $item) {
 			/* only hostname-class fields may legitimately need percent
@@ -3289,13 +3325,22 @@ function get_full_script_path(int $local_data_id) : mixed {
 				$value = "''";
 			}
 
-			$full_path = str_replace('<' . $item['data_name'] . '>', $value, $full_path);
+			$escaped_values[$item['data_name']] = $value;
 		}
 	}
 
-	$search    = ['<path_cacti>', '<path_snmpget>', '<path_php_binary>'];
-	$replace   = [CACTI_PATH_BASE, read_config_option('path_snmpget'), read_config_option('path_php_binary')];
-	$full_path = str_replace($search, $replace, $full_path);
+	/* the well-known path tokens resolve to trusted configuration values; field
+	 * names take precedence so behaviour matches the historical field-first order */
+	$escaped_values += [
+		'path_cacti'      => CACTI_PATH_BASE,
+		'path_snmpget'    => read_config_option('path_snmpget'),
+		'path_php_binary' => read_config_option('path_php_binary'),
+	];
+
+	/* single-pass substitution over the original template prevents a field whose
+	 * value contains another field's <token> from re-injecting an escaped payload
+	 * into an already-quoted region (GHSA-fq9x-x3vf-3vf2) */
+	$full_path = substitute_script_path($full_path, $escaped_values);
 
 	/* sometimes a certain input value will not have anything entered... null out these fields
 	in the input string so we don't mess up the script */

lib/html.php

@@ -3265,7 +3265,7 @@ function html_common_header(string $title, string $selectedTheme = '') : void {
 	<meta http-equiv="Content-Security-Policy" content="default-src *; img-src 'self' https://api.qrserver.com <?php print $alternates; ?> data: blob:; style-src 'self' 'unsafe-inline' <?php print $alternates; ?>; <?php print $script_src; ?>; worker-src 'self' <?php print $alternates; ?>;">
 
 
-	<title><?php print $title; ?></title>
+	<title><?php print html_escape($title); ?></title>
 	<meta http-equiv='Content-Type' content='text/html;charset=utf-8'>
 	<link rel='manifest' href='/manifest.json'>
 	<script type='text/javascript' <?php print CactiSecureHeaders::getNonceAttribute(); ?>>

tests/Unit/Security/Batch11SecurityRegressionTest.php

@@ -0,0 +1,113 @@
+<?php
+/*
+ +-------------------------------------------------------------------------+
+ | Copyright (C) 2004-2026 The Cacti Group                                 |
+ |                                                                         |
+ | This program is free software; you can redistribute it and/or           |
+ | modify it under the terms of the GNU General Public License             |
+ | as published by the Free Software Foundation; either version 2          |
+ | of the License, or (at your option) any later version.                  |
+ +-------------------------------------------------------------------------+
+ | Cacti: The Complete RRDtool-based Graphing Solution                     |
+ +-------------------------------------------------------------------------+
+*/
+
+/*
+ * Batch-11 security consolidation regressions (develop), guarding the four
+ * sinks fixed in PR #8165:
+ *
+ *   GHSA-fq9x-x3vf-3vf2  single-pass data-input <field> substitution
+ *   GHSA-hh7r-464j-rf39  Content-Disposition filename sanitization
+ *   GHSA-v99q-57w7-mpmg  HTML <title> escaping
+ *   GHSA-f5f5-h85v-6m3h  user administration tooltip text encoding
+ */
+
+$root = dirname(__DIR__, 3);
+
+$functionsSource = file_get_contents($root . '/lib/functions.php');
+$xportSource     = file_get_contents($root . '/graph_xport.php');
+$htmlSource      = file_get_contents($root . '/lib/html.php');
+$userAdminSource = file_get_contents($root . '/user_admin.php');
+$userGroupSource = file_get_contents($root . '/user_group_admin.php');
+
+// =====================================================================
+// GHSA-fq9x: single-pass <field> token substitution
+// =====================================================================
+
+test('GHSA-fq9x: substitute_script_path substitutes tokens in a single pass', function () use ($functionsSource) {
+	expect($functionsSource)->toContain('function substitute_script_path(');
+	expect($functionsSource)->toContain("preg_replace_callback('/<([A-Za-z0-9_]+)>/',");
+	expect($functionsSource)->toContain('array_key_exists($matches[1], $escaped_values)');
+});
+
+test('GHSA-fq9x: both path builders drop the iterative str_replace and call the helper', function () use ($functionsSource) {
+	expect($functionsSource)->not->toContain("\$full_path = str_replace('<' . \$item['data_name'] . '>', \$value, \$full_path);");
+	expect(substr_count($functionsSource, '$full_path = substitute_script_path($full_path, $escaped_values);'))->toBe(2);
+});
+
+/*
+ * Behavioral proof. The develop unit bootstrap loads lib/functions.php via
+ * include/global.php, so substitute_script_path() is normally already defined.
+ * As a defensive fallback (and to document the Test-only nature of the eval),
+ * the real function body is regex-extracted from this repo's own source and
+ * eval()'d only when it is not already present -- the extract-and-eval pattern
+ * used elsewhere in the suite. Input is this repo's source, never user input.
+ */
+if (!function_exists('substitute_script_path')) {
+	preg_match('/\nfunction substitute_script_path\b.*?\n\}/s', "\n" . $functionsSource, $sspMatch);
+
+	if (!empty($sspMatch)) {
+		eval($sspMatch[0]);
+	}
+}
+
+test('GHSA-fq9x: a field value that contains another field token is not re-substituted', function () {
+	expect(function_exists('substitute_script_path'))->toBeTrue();
+
+	$result = substitute_script_path('<field1>', [
+		'field1' => '<arg2>',
+		'arg2'   => 'PAYLOAD',
+	]);
+
+	expect($result)->toBe('<arg2>');
+	expect($result)->not->toContain('PAYLOAD');
+});
+
+test('GHSA-fq9x: known tokens are replaced once and unknown tokens remain literal', function () {
+	expect(substitute_script_path('<unknown>', []))->toBe('<unknown>');
+	expect(substitute_script_path('<f>-<f>', ['f' => "'v'"]))->toBe("'v'-'v'");
+	expect(substitute_script_path('<a><b>', ['a' => 'X', 'b' => 'Y']))->toBe('XY');
+});
+
+// =====================================================================
+// GHSA-hh7r: Content-Disposition filename sanitization
+// =====================================================================
+
+test('GHSA-hh7r: the export filename strips CR/LF/quote/backslash from the title', function () use ($xportSource) {
+	expect($xportSource)->toContain('$filename = str_replace(["\r", "\n", \'"\', \'\\\\\'], \'\', (string) $xport_array[\'meta\'][\'title_cache\']) . \'.csv\';');
+	// the raw, unsanitized concatenation must be gone.
+	expect($xportSource)->not->toContain('$filename = $xport_array[\'meta\'][\'title_cache\'] . \'.csv\';');
+});
+
+// =====================================================================
+// GHSA-v99q: HTML <title> escaping
+// =====================================================================
+
+test('GHSA-v99q: the page title is html-escaped before entering the <title> element', function () use ($htmlSource) {
+	expect($htmlSource)->toContain('<title><?php print html_escape($title); ?></title>');
+	expect($htmlSource)->not->toContain('<title><?php print $title; ?></title>');
+});
+
+// =====================================================================
+// GHSA-f5f5: user administration tooltip text encoding
+// =====================================================================
+
+test('GHSA-f5f5: the user-admin tooltip renders data-tooltip as escaped text', function () use ($userAdminSource) {
+	expect($userAdminSource)->toContain("return $('<div>').text($(this).attr('data-tooltip') || '').html();");
+	expect($userAdminSource)->not->toContain("return $(this).attr('data-tooltip');");
+});
+
+test('GHSA-f5f5: the user-group-admin tooltip also renders data-tooltip as escaped text', function () use ($userGroupSource) {
+	expect($userGroupSource)->toContain("return $('<div>').text($(this).attr('data-tooltip') || '').html();");
+	expect($userGroupSource)->not->toContain("return $(this).attr('data-tooltip');");
+});

user_admin.php

@@ -966,7 +966,9 @@ function graph_perms_edit(string $tab, string $header_label) : void {
 				$(document).tooltip({
 					items: '[data-tooltip]',
 					content: function() {
-						return $(this).attr('data-tooltip');
+						// Render tooltip text as escaped content to prevent any markup
+						// in data-tooltip from being interpreted as HTML.
+						return $('<div>').text($(this).attr('data-tooltip') || '').html();
 					}
 				});
 			});

user_group_admin.php

@@ -963,7 +963,9 @@ function user_group_graph_perms_edit(string $tab, string $header_label) : void {
 				$(document).tooltip({
 					items: '[data-tooltip]',
 					content: function() {
-						return $(this).attr('data-tooltip');
+						// Render tooltip text as escaped content to prevent any markup
+						// in data-tooltip from being interpreted as HTML.
+						return $('<div>').text($(this).attr('data-tooltip') || '').html();
 					}
 				});
 			});