hardening: migrate monitor SQL helpers to prepared variants

Author: somethingwithproof

setup.php

@@ -157,9 +157,9 @@ function monitor_device_table_bottom() {
 }
 
 function plugin_monitor_uninstall() {
-	db_execute('DROP TABLE IF EXISTS plugin_monitor_notify_history');
-	db_execute('DROP TABLE IF EXISTS plugin_monitor_reboot_history');
-	db_execute('DROP TABLE IF EXISTS plugin_monitor_uptime');
+	db_execute_prepared('DROP TABLE IF EXISTS plugin_monitor_notify_history');
+	db_execute_prepared('DROP TABLE IF EXISTS plugin_monitor_reboot_history');
+	db_execute_prepared('DROP TABLE IF EXISTS plugin_monitor_uptime');
 }
 
 function plugin_monitor_page_head() {
@@ -203,7 +203,12 @@ function monitor_check_upgrade() {
 
 	$info    = plugin_monitor_version();
 	$current = $info['version'];
-	$old     = db_fetch_cell('SELECT version FROM plugin_config WHERE directory = "monitor"');
+	$old     = db_fetch_cell_prepared(
+		'SELECT version
+		FROM plugin_config
+		WHERE directory = ?',
+		['monitor']
+	);
 
 	if ($current != $old) {
 		monitor_setup_table();
@@ -319,9 +324,16 @@ function monitor_device_action_execute($action) {
 }
 
 function monitor_device_remove($devices) {
-	db_execute('DELETE FROM plugin_monitor_notify_history WHERE host_id IN(' . implode(',', $devices) . ')');
-	db_execute('DELETE FROM plugin_monitor_reboot_history WHERE host_id IN(' . implode(',', $devices) . ')');
-	db_execute('DELETE FROM plugin_monitor_uptime WHERE host_id IN(' . implode(',', $devices) . ')');
+	if (!cacti_sizeof($devices)) {
+		return $devices;
+	}
+
+	$devices      = array_map('intval', $devices);
+	$placeholders = implode(',', array_fill(0, cacti_sizeof($devices), '?'));
+
+	db_execute_prepared("DELETE FROM plugin_monitor_notify_history WHERE host_id IN($placeholders)", $devices);
+	db_execute_prepared("DELETE FROM plugin_monitor_reboot_history WHERE host_id IN($placeholders)", $devices);
+	db_execute_prepared("DELETE FROM plugin_monitor_uptime WHERE host_id IN($placeholders)", $devices);
 
 	return $devices;
 }

tests/test_prepared_statements.php

@@ -0,0 +1,83 @@
+<?php
+
+function assert_contains(string $haystack, string $needle, string $message): void {
+	if (strpos($haystack, $needle) === false) {
+		fwrite(STDERR, $message . PHP_EOL);
+		exit(1);
+	}
+}
+
+function assert_not_contains(string $haystack, string $needle, string $message): void {
+	if (strpos($haystack, $needle) !== false) {
+		fwrite(STDERR, $message . PHP_EOL);
+		exit(1);
+	}
+}
+
+$setup = file_get_contents(__DIR__ . '/../setup.php');
+if ($setup === false) {
+	fwrite(STDERR, "Unable to read setup.php\n");
+	exit(1);
+}
+
+assert_contains(
+	$setup,
+	"db_fetch_cell_prepared(\n\t\t'SELECT version",
+	'Expected plugin version lookup to use db_fetch_cell_prepared().'
+);
+
+assert_contains(
+	$setup,
+	"WHERE directory = ?',\n\t\t['monitor']",
+	'Expected monitor directory lookup to use a bound placeholder.'
+);
+
+assert_contains(
+	$setup,
+	"db_execute_prepared('DROP TABLE IF EXISTS plugin_monitor_notify_history');",
+	'Expected uninstall notify-history drop to use db_execute_prepared().'
+);
+
+assert_contains(
+	$setup,
+	"db_execute_prepared('DROP TABLE IF EXISTS plugin_monitor_reboot_history');",
+	'Expected uninstall reboot-history drop to use db_execute_prepared().'
+);
+
+assert_contains(
+	$setup,
+	"db_execute_prepared('DROP TABLE IF EXISTS plugin_monitor_uptime');",
+	'Expected uninstall uptime drop to use db_execute_prepared().'
+);
+
+assert_contains(
+	$setup,
+	'$placeholders = implode(\',\', array_fill(0, cacti_sizeof($devices), \'?\'));',
+	'Expected monitor_device_remove() to build placeholder list for IN clause.'
+);
+
+assert_contains(
+	$setup,
+	'db_execute_prepared("DELETE FROM plugin_monitor_notify_history WHERE host_id IN($placeholders)", $devices);',
+	'Expected notify history delete to use db_execute_prepared() with placeholders.'
+);
+
+assert_contains(
+	$setup,
+	'db_execute_prepared("DELETE FROM plugin_monitor_reboot_history WHERE host_id IN($placeholders)", $devices);',
+	'Expected reboot history delete to use db_execute_prepared() with placeholders.'
+);
+
+assert_contains(
+	$setup,
+	'db_execute_prepared("DELETE FROM plugin_monitor_uptime WHERE host_id IN($placeholders)", $devices);',
+	'Expected uptime delete to use db_execute_prepared() with placeholders.'
+);
+
+assert_not_contains(
+	$setup,
+	'db_execute(\'DELETE FROM plugin_monitor_notify_history WHERE host_id IN(\' . implode(\',\', $devices) . \')\');',
+	'Raw notify-history delete should not remain.'
+);
+
+echo "OK\n";