security: fix SQLi, XSS, credential exposure, and insecure deserialization

[somethingwithproof]

Summary

Comprehensive security hardening using existing Cacti core helpers. No new code needed; all fixes use db_qstr(), html_escape_request_var(), html_escape(), and cacti_unserialize().

Critical fixes:

• Strip 9 SNMP credential columns from guest-accessible CSV export (GHSA-9829-w9mx-2cgm)
• Replace bare unserialize() with cacti_unserialize() + array validation on guest page (mactrack_view_macs.php)
• Use db_qstr() in CSV import value/WHERE construction (mactrack_devices.php, mactrack_device_types.php)

SQL injection fixes (27 sites across 9 files):

• Replace all LIKE '%" . get_request_var('filter') . "%' with LIKE " . db_qstr(...) in mactrack_view_sites.php, mactrack_view_devices.php, mactrack_vendormacs.php, mactrack_sites.php, mactrack_macwatch.php, mactrack_snmp.php, mactrack_device_types.php, mactrack_devices.php, mactrack_macauth.php
• Fix mactrack_create_sql_filter() in lib/mactrack_functions.php

XSS fixes:

• Replace get_request_var('filter') with html_escape_request_var('filter') in 11 HTML input value attributes
• Wrap SNMP response variables with html_escape() in mactrack_devices.php

Addresses

• GHSA-9829-w9mx-2cgm (SNMP credentials in guest CSV export)

Test plan

• Verify device CSV export no longer contains SNMP credentials
• Verify filter search works across all view pages
• Verify CSV import of devices and device types works
• Verify MAC address actions work on view_macs page
• Verify SNMP test output displays correctly with special characters

[Copilot]

Pull request overview

Security hardening for the Mactrack plugin by reducing injection exposure (SQLi/XSS) across multiple report/admin pages and by removing sensitive SNMP credentials from guest-accessible exports.

Changes:

• Replaced unsafe LIKE '%...%' SQL string concatenation with db_qstr()-quoted patterns across multiple filters (including mactrack_create_sql_filter()).
• Escaped user-provided filter values in HTML inputs via html_escape_request_var('filter') to prevent reflected XSS.
• Hardened risky data handling by removing SNMP credential fields from device CSV export, replacing unserialize() with cacti_unserialize(), and HTML-escaping SNMP test output.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
mactrack_view_sites.php	Uses db_qstr() for site/device-type filter SQL LIKE clauses.
mactrack_view_macs.php	Replaces unserialize() with cacti_unserialize() + basic type check; escapes filter input value.
mactrack_view_interfaces.php	Escapes filter input value to prevent reflected XSS.
mactrack_view_devices.php	Removes SNMP credential columns from guest CSV export; hardens filter SQL; escapes filter input value.
mactrack_view_arp.php	Escapes filter input value to prevent reflected XSS.
mactrack_vendormacs.php	Uses db_qstr() for filter SQL; escapes filter input value.
mactrack_snmp.php	Uses db_qstr() for filter SQL; escapes filter input value.
mactrack_sites.php	Uses db_qstr() for filter SQL LIKE clauses.
mactrack_macwatch.php	Uses db_qstr() for filter SQL; escapes filter input value.
mactrack_macauth.php	Uses db_qstr() for filter SQL; escapes filter input value.
mactrack_devices.php	Uses db_qstr() in CSV import INSERT/WHERE construction; HTML-escapes SNMP test output; hardens filter SQL.
mactrack_device_types.php	Uses db_qstr() in CSV import INSERT/WHERE construction and filter SQL; escapes filter input value.
lib/mactrack_functions.php	Updates mactrack_create_sql_filter() to use db_qstr(); escapes filter input value in site filter UI.

Comments suppressed due to low confidence (2)

mactrack_view_devices.php:130

• mactrack_view_export_devices() is manually building CSV rows via string concatenation without escaping embedded quotes/newlines/commas in fields like site_name, device_name, or notes. This can generate malformed CSV and also enables CSV formula injection if any exported field begins with =, +, -, or @ and is opened in a spreadsheet. Consider switching to fputcsv() (or an existing Cacti CSV helper) and applying a spreadsheet-safe prefix/escaping for cell values.

	array_push($xport_array, 'site_id, site_name, device_id, device_name, notes, ' .
		'hostname, snmp_version, ' .
		'snmp_port, snmp_timeout, snmp_retries, max_oids, snmp_sysName, snmp_sysLocation, ' .
		'snmp_sysContact, snmp_sysObjectID, snmp_sysDescr, snmp_sysUptime, ' .
		'ignorePorts, scan_type, disabled, ports_total, ports_active, ' .
		'ports_trunk, macs_active, last_rundate, last_runduration');

	if (cacti_sizeof($devices)) {
		foreach ($devices as $device) {
			array_push($xport_array,'"' .
			$device['site_id'] . '","' . $device['site_name'] . '","' .
			$device['device_id'] . '","' . $device['device_name'] . '","' .
			$device['notes'] . '","' . $device['hostname'] . '","' .
			$device['snmp_version'] . '","' .
			$device['snmp_port'] . '","' . $device['snmp_timeout'] . '","' .
			$device['snmp_retries'] . '","' . $device['max_oids'] . '","' .
			$device['snmp_sysName'] . '","' . $device['snmp_sysLocation'] . '","' .
			$device['snmp_sysContact'] . '","' . $device['snmp_sysObjectID'] . '","' .
			$device['snmp_sysDescr'] . '","' . $device['snmp_sysUptime'] . '","' .
			$device['ignorePorts'] . '","' . $device['scan_type'] . '","' .
			$device['disabled'] . '","' . $device['ports_total'] . '","' .
			$device['ports_active'] . '","' . $device['ports_trunk'] . '","' .
			$device['macs_active'] . '","' . $device['last_rundate'] . '","' .
			$device['last_runduration'] . '"');

mactrack_devices.php:752

• In the device CSV import path, the code strips all double quotes from each field (str_replace('"', '', $line_item)) and later relies on explode(',', ...) parsing earlier in the function. This is not valid CSV handling (quoted fields, embedded commas, and escaped quotes will be corrupted). Since this area is being touched for security hardening, consider switching to a proper CSV parser (str_getcsv()/fgetcsv()) and removing the blanket quote stripping.

						if (in_array($j, $insert_columns, true)) {
							$line_item = trim(str_replace('"', '', $line_item));

							if (!$first_column) {
								$save_value .= ',';

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The filter SQL in mactrack_get_devices() is assembled using a multi-line string literal that spans lines 975–977. While it works, it’s much harder to read and differs from the pattern used elsewhere in the plugin (explicit concatenation with " OR "). Consider rewriting this as a single-line concatenation (or building an array of clauses and implode(' OR ', ...)) to avoid accidental quoting/whitespace mistakes in future edits.

Suggested change

	$sql_where = ($sql_where != '' ? ' AND ' : 'WHERE ') . "(mtd.hostname LIKE " . db_qstr('%' . get_request_var('filter') . '%') . "
	OR mtd.device_name LIKE " . db_qstr('%' . get_request_var('filter') . '%') . "
	OR mtd.notes LIKE " . db_qstr('%' . get_request_var('filter') . '%') . ")";
	$filter = db_qstr('%' . get_request_var('filter') . '%');
	$sql_where = ($sql_where != '' ? ' AND ' : 'WHERE ') .
	'(mtd.hostname LIKE ' . $filter .
	' OR mtd.device_name LIKE ' . $filter .
	' OR mtd.notes LIKE ' . $filter . ')';

[Copilot]

This action handler now uses cacti_unserialize() directly, but the same file also uses sanitize_unserialize_selected_items() for other actions. Using two different unserialize/validation paths for selected_items makes behavior inconsistent and increases the chance of missing validation in one path. Consider routing this through the same helper (or otherwise centralizing the validation) so all bulk-action entry points enforce the same rules.

Suggested change

	$selected_items = cacti_unserialize(stripslashes(get_nfilter_request_var('selected_items')));
	if (!is_array($selected_items)) {
	header('Location: mactrack_view_macs.php');
	exit;
	}
	$selected_items = sanitize_unserialize_selected_items();

[Copilot]

Same as the device import: this CSV import logic strips all double quotes from fields and depends on a simplistic comma split earlier, which will corrupt valid CSV containing quoted fields, embedded commas, or escaped quotes. Consider using a proper CSV parser (str_getcsv()/fgetcsv()) and removing the blanket quote stripping so imports are correct and predictable.

Suggested change

	$line_item = trim(str_replace('"', '', $line_item));
	$line_item = trim($line_item);