From b9f2c5faba39e19a7b0e2b3da200b733283d1024 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 17 May 2026 02:18:22 -0700 Subject: [PATCH 1/3] fix(mactrack): fix 10 logic bugs in ARP lookup, resolver, and VLAN port scanning Signed-off-by: Thomas Vincent --- lib/mactrack_extreme.php | 1 - lib/mactrack_foundry.php | 1 - lib/mactrack_functions.php | 8 ++++---- lib/mactrack_hp.php | 1 - lib/mactrack_hp_ng.php | 3 +-- lib/mactrack_hp_ngi.php | 1 - lib/mactrack_juniper.php | 1 - mactrack_devices.php | 2 +- mactrack_resolver.php | 8 ++++---- mactrack_scanner.php | 2 +- poller_mactrack.php | 6 +++--- 11 files changed, 14 insertions(+), 20 deletions(-) diff --git a/lib/mactrack_extreme.php b/lib/mactrack_extreme.php index 4682983..6d24094 100644 --- a/lib/mactrack_extreme.php +++ b/lib/mactrack_extreme.php @@ -86,7 +86,6 @@ function get_extreme_switch_ports($site, &$device, $lowPort = 0, $highPort = 0, foreach ($vlan_ids as $vlan_index => $vlan_id) { $active_vlans[$i]['vlan_id'] = $vlan_id; $active_vlans[$i]['vlan_name'] = $vlan_names[$vlan_index]; - $active_vlans++; mactrack_debug('VLAN ID = ' . $active_vlans[$i]['vlan_id'] . ' VLAN Name = ' . $active_vlans[$i]['vlan_name']); $i++; } diff --git a/lib/mactrack_foundry.php b/lib/mactrack_foundry.php index 3440e14..37bc05f 100644 --- a/lib/mactrack_foundry.php +++ b/lib/mactrack_foundry.php @@ -104,7 +104,6 @@ function get_foundry_switch_ports($site, &$device, $lowPort = 0, $highPort = 0) foreach ($vlan_ids as $vlan_id => $vlan_name) { $active_vlans[$i]['vlan_id'] = $vlan_id; $active_vlans[$i]['vlan_name'] = $vlan_name; - $active_vlans++; mactrack_debug('VLAN ID = ' . $active_vlans[$i]['vlan_id'] . ' VLAN Name = ' . $active_vlans[$i]['vlan_name']); $i++; } diff --git a/lib/mactrack_functions.php b/lib/mactrack_functions.php index a70dfca..d1224ad 100644 --- a/lib/mactrack_functions.php +++ b/lib/mactrack_functions.php @@ -2147,7 +2147,7 @@ function xform_mac_address($mac_address) { $mac_address = $mac; } - $mac_address = str_replace(':', '', $max_address); + $mac_address = str_replace(':', '', $mac_address); return strtoupper($mac_address); } @@ -2838,13 +2838,13 @@ function get_netscreen_arp_table($site, &$device) { $ifIntcount = 0; } - if ($ifIntcount != 0) { + if ($ifIntcount == 0) { $atifIndexes = xform_indexed_data('.1.3.6.1.2.1.4.22.1.1', $device, 5); } mactrack_debug(__('atifIndexes data collection complete', 'mactrack')); // get the atPhysAddress for the device - if ($ifIntcount != 0) { + if ($ifIntcount == 0) { $atPhysAddress = xform_indexed_data('.1.3.6.1.2.1.4.22.1.2', $device, 5, true); } else { $atPhysAddress = xform_indexed_data('.1.3.6.1.2.1.3.1.1.2', $device, 6, true); @@ -2863,7 +2863,7 @@ function get_netscreen_arp_table($site, &$device) { mactrack_debug(__('atPhysAddress data collection complete', 'mactrack')); // get the atPhysAddress for the device - if ($ifIntcount != 0) { + if ($ifIntcount == 0) { $atNetAddress = xform_indexed_data('.1.3.6.1.2.1.4.22.1.3', $device, 5); } else { $atNetAddress = xform_indexed_data('.1.3.6.1.2.1.3.1.1.3', $device, 6); diff --git a/lib/mactrack_hp.php b/lib/mactrack_hp.php index da9d18e..036c33d 100644 --- a/lib/mactrack_hp.php +++ b/lib/mactrack_hp.php @@ -75,7 +75,6 @@ function get_procurve_switch_ports($site, &$device, $lowPort = 0, $highPort = 0) foreach ($vlan_ids as $vlan_id => $vlan_name) { $active_vlans[$i]['vlan_id'] = $vlan_id; $active_vlans[$i]['vlan_name'] = $vlan_name; - $active_vlans++; $i++; } diff --git a/lib/mactrack_hp_ng.php b/lib/mactrack_hp_ng.php index 1bf274b..6bca5b1 100644 --- a/lib/mactrack_hp_ng.php +++ b/lib/mactrack_hp_ng.php @@ -72,7 +72,6 @@ function get_procurve_ng_switch_ports($site, &$device, $lowPort = 0, $highPort = foreach ($vlan_ids as $vlan_id => $vlan_name) { $active_vlans[$i]['vlan_id'] = $vlan_id; $active_vlans[$i]['vlan_name'] = $vlan_name; - $active_vlans++; $i++; } @@ -91,7 +90,7 @@ function get_procurve_ng_switch_ports($site, &$device, $lowPort = 0, $highPort = foreach ($port_results as $port_result) { $ifIndex = $port_result['port_number']; $ifType = isset($ifInterfaces[$ifIndex]['ifType']) ? $ifInterfaces[$ifIndex]['ifType'] : ''; - $ifName = isset($ifInterfaces['ifAlias'][$ifIndex]) ? $ifInterfaces['ifAlias'][$ifIndex] : ''; + $ifName = isset($ifInterfaces[$ifIndex]['ifAlias']) ? $ifInterfaces[$ifIndex]['ifAlias'] : ''; $portName = $ifName; $portTrunkStatus = isset($ifInterfaces[$ifIndex]['trunkPortState']) ? $ifInterfaces[$ifIndex]['trunkPortState'] : ''; diff --git a/lib/mactrack_hp_ngi.php b/lib/mactrack_hp_ngi.php index d4a3309..8a22f74 100644 --- a/lib/mactrack_hp_ngi.php +++ b/lib/mactrack_hp_ngi.php @@ -89,7 +89,6 @@ function get_procurve_ngi_switch_ports($site, &$device, $lowPort = 0, $highPort foreach ($vlan_ids as $vlan_id => $vlan_name) { $active_vlans[$i]['vlan_id'] = $vlan_id; $active_vlans[$i]['vlan_name'] = $vlan_name; - $active_vlans++; $i++; } diff --git a/lib/mactrack_juniper.php b/lib/mactrack_juniper.php index 389f56a..5bf14ad 100644 --- a/lib/mactrack_juniper.php +++ b/lib/mactrack_juniper.php @@ -97,7 +97,6 @@ function get_JEX_switch_ports($site, &$device, $lowPort = 0, $highPort = 0) { foreach ($vlan_ids as $vlan_id => $vlan_num) { $active_vlans[$vlan_id]['vlan_id'] = $vlan_num; $active_vlans[$vlan_id]['vlan_name'] = mactrack_arr_key($vlan_names, $vlan_id); - $active_vlans++; $i++; } diff --git a/mactrack_devices.php b/mactrack_devices.php index 5c895ad..3c9f02f 100644 --- a/mactrack_devices.php +++ b/mactrack_devices.php @@ -170,7 +170,7 @@ function form_mactrack_actions() { if (isset_request_var("t_$field_name") && preg_match('/^ignorePorts/', $field_name)) { db_execute_prepared("UPDATE mac_track_devices SET $field_name = ? - WHERE id = ?", + WHERE device_id = ?", [get_request_var($field_name), $selected_items[$i]]); } } diff --git a/mactrack_resolver.php b/mactrack_resolver.php index 7f9bf7a..aebc394 100644 --- a/mactrack_resolver.php +++ b/mactrack_resolver.php @@ -80,7 +80,7 @@ if (cacti_sizeof($parms)) { foreach ($parms as $parameter) { - if (strpos($parameter, '=')) { + if (strpos($parameter, '=') !== false) { [$arg, $value] = explode('=', $parameter); } else { $arg = $parameter; @@ -153,11 +153,11 @@ } if (cacti_sizeof($nameservers)) { - $use_resolver = false; - $resolver = false; -} else { $use_resolver = true; $resolver = new Net_DNS2_Resolver(['nameservers' => $nameservers]); +} else { + $use_resolver = false; + $resolver = false; } // if more than 15 second is nothing to do, ending diff --git a/mactrack_scanner.php b/mactrack_scanner.php index 4b03f44..e59af53 100644 --- a/mactrack_scanner.php +++ b/mactrack_scanner.php @@ -62,7 +62,7 @@ if (cacti_sizeof($parms)) { foreach ($parms as $parameter) { - if (strpos($parameter, '=')) { + if (strpos($parameter, '=') !== false) { [$arg, $value] = explode('=', $parameter); } else { $arg = $parameter; diff --git a/poller_mactrack.php b/poller_mactrack.php index 3a6ad8b..f96b49d 100644 --- a/poller_mactrack.php +++ b/poller_mactrack.php @@ -86,7 +86,7 @@ if (cacti_sizeof($parms)) { foreach ($parms as $parameter) { - if (strpos($parameter, '=')) { + if (strpos($parameter, '=') !== false) { [$arg, $value] = explode('=', $parameter); } else { $arg = $parameter; @@ -720,7 +720,7 @@ function collect_mactrack_data($start, $site_id = 0) { WHERE site_id = ? AND device_id = ? AND mac_address = ?', - [$macs['ip_address'], $port['site_id'], $port['device_id'] . $port['mac_address']]); + [$macs['ip_address'], $port['site_id'], $port['device_id'], $port['mac_address']]); } } } @@ -998,7 +998,7 @@ function collect_mactrack_data($start, $site_id = 0) { $last_macauth_time = read_config_option('mt_last_macauth_time'); // if it's time to e-mail - if (($last_macauth_time + ($mac_auth_frequency * 60) > time()) || + if (($last_macauth_time + ($mac_auth_frequency * 60) < time()) || ($mac_auth_frequency == 0)) { mactrack_process_mac_auth_report($mac_auth_frequency, $last_macauth_time); } From 5e316080cac0acd98f9d4fa93988fd3c67a59cc1 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 17 May 2026 02:26:51 -0700 Subject: [PATCH 2/3] fix(mactrack): revert ARP MIB condition to != 0 Confirmed != 0 is correct: when device responds to AT-MIB probe (ifIntcount > 0), use newer ipNetToMedia OIDs for collection. Signed-off-by: Thomas Vincent --- lib/mactrack_functions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/mactrack_functions.php b/lib/mactrack_functions.php index d1224ad..be8d074 100644 --- a/lib/mactrack_functions.php +++ b/lib/mactrack_functions.php @@ -2838,13 +2838,13 @@ function get_netscreen_arp_table($site, &$device) { $ifIntcount = 0; } - if ($ifIntcount == 0) { + if ($ifIntcount != 0) { $atifIndexes = xform_indexed_data('.1.3.6.1.2.1.4.22.1.1', $device, 5); } mactrack_debug(__('atifIndexes data collection complete', 'mactrack')); // get the atPhysAddress for the device - if ($ifIntcount == 0) { + if ($ifIntcount != 0) { $atPhysAddress = xform_indexed_data('.1.3.6.1.2.1.4.22.1.2', $device, 5, true); } else { $atPhysAddress = xform_indexed_data('.1.3.6.1.2.1.3.1.1.2', $device, 6, true); @@ -2863,7 +2863,7 @@ function get_netscreen_arp_table($site, &$device) { mactrack_debug(__('atPhysAddress data collection complete', 'mactrack')); // get the atPhysAddress for the device - if ($ifIntcount == 0) { + if ($ifIntcount != 0) { $atNetAddress = xform_indexed_data('.1.3.6.1.2.1.4.22.1.3', $device, 5); } else { $atNetAddress = xform_indexed_data('.1.3.6.1.2.1.3.1.1.3', $device, 6); From 961ea1cf75c443e8b236da5679e0e96027860e62 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Sun, 17 May 2026 03:23:49 -0700 Subject: [PATCH 3/3] test: add Pest v1 security test scaffold from #328 Signed-off-by: Thomas Vincent --- tests/Pest.php | 14 ++ tests/Security/Php74CompatibilityTest.php | 108 ++++++++++ .../PreparedStatementConsistencyTest.php | 66 ++++++ tests/Security/SetupStructureTest.php | 36 ++++ tests/bootstrap.php | 200 ++++++++++++++++++ 5 files changed, 424 insertions(+) create mode 100644 tests/Pest.php create mode 100644 tests/Security/Php74CompatibilityTest.php create mode 100644 tests/Security/PreparedStatementConsistencyTest.php create mode 100644 tests/Security/SetupStructureTest.php create mode 100644 tests/bootstrap.php diff --git a/tests/Pest.php b/tests/Pest.php new file mode 100644 index 0000000..e2132a3 --- /dev/null +++ b/tests/Pest.php @@ -0,0 +1,14 @@ +toBe(0, + "{$relativeFile} uses str_contains() which requires PHP 8.0" + ); + } + }); + + it('does not use str_starts_with (PHP 8.0)', function () use ($files) { + foreach ($files as $relativeFile) { + $path = realpath(__DIR__ . '/../../' . $relativeFile); + + if ($path === false) { + continue; + } + + $contents = file_get_contents($path); + + if ($contents === false) { + continue; + } + + expect(preg_match('/\bstr_starts_with\s*\(/', $contents))->toBe(0, + "{$relativeFile} uses str_starts_with() which requires PHP 8.0" + ); + } + }); + + it('does not use str_ends_with (PHP 8.0)', function () use ($files) { + foreach ($files as $relativeFile) { + $path = realpath(__DIR__ . '/../../' . $relativeFile); + + if ($path === false) { + continue; + } + + $contents = file_get_contents($path); + + if ($contents === false) { + continue; + } + + expect(preg_match('/\bstr_ends_with\s*\(/', $contents))->toBe(0, + "{$relativeFile} uses str_ends_with() which requires PHP 8.0" + ); + } + }); + + it('does not use nullsafe operator (PHP 8.0)', function () use ($files) { + foreach ($files as $relativeFile) { + $path = realpath(__DIR__ . '/../../' . $relativeFile); + + if ($path === false) { + continue; + } + + $contents = file_get_contents($path); + + if ($contents === false) { + continue; + } + + expect(preg_match('/\?->/', $contents))->toBe(0, + "{$relativeFile} uses nullsafe operator which requires PHP 8.0" + ); + } + }); +}); diff --git a/tests/Security/PreparedStatementConsistencyTest.php b/tests/Security/PreparedStatementConsistencyTest.php new file mode 100644 index 0000000..5e3e471 --- /dev/null +++ b/tests/Security/PreparedStatementConsistencyTest.php @@ -0,0 +1,66 @@ +toBe(0, + "File {$relativeFile} contains raw (unprepared) DB calls" + ); + } + }); +}); diff --git a/tests/Security/SetupStructureTest.php b/tests/Security/SetupStructureTest.php new file mode 100644 index 0000000..e789db3 --- /dev/null +++ b/tests/Security/SetupStructureTest.php @@ -0,0 +1,36 @@ +toContain('function plugin_mactrack_install'); + }); + + it('defines plugin_mactrack_version function', function () use ($source) { + expect($source)->toContain('function plugin_mactrack_version'); + }); + + it('defines plugin_mactrack_uninstall function', function () use ($source) { + expect($source)->toContain('function plugin_mactrack_uninstall'); + }); + + it('returns version array with name key', function () use ($source) { + expect($source)->toMatch('/[\'\""]name[\'\""]\s*=>/'); + }); + + it('returns version array with version key', function () use ($source) { + expect($source)->toMatch('/[\'\""]version[\'\""]\s*=>/'); + }); +}); diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..0aa22f2 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,200 @@ + 'db_execute', 'sql' => $sql, 'params' => array()); + return true; + } +} + +if (!function_exists('db_execute_prepared')) { + function db_execute_prepared($sql, $params = array()) { + $GLOBALS['__test_db_calls'][] = array('fn' => 'db_execute_prepared', 'sql' => $sql, 'params' => $params); + return true; + } +} + +if (!function_exists('db_fetch_assoc')) { + function db_fetch_assoc($sql) { + return array(); + } +} + +if (!function_exists('db_fetch_assoc_prepared')) { + function db_fetch_assoc_prepared($sql, $params = array()) { + return array(); + } +} + +if (!function_exists('db_fetch_row')) { + function db_fetch_row($sql) { + return array(); + } +} + +if (!function_exists('db_fetch_row_prepared')) { + function db_fetch_row_prepared($sql, $params = array()) { + return array(); + } +} + +if (!function_exists('db_fetch_cell')) { + function db_fetch_cell($sql) { + return ''; + } +} + +if (!function_exists('db_fetch_cell_prepared')) { + function db_fetch_cell_prepared($sql, $params = array()) { + return ''; + } +} + +if (!function_exists('db_index_exists')) { + function db_index_exists($table, $index) { + return false; + } +} + +if (!function_exists('db_column_exists')) { + function db_column_exists($table, $column) { + return false; + } +} + +if (!function_exists('api_plugin_db_add_column')) { + function api_plugin_db_add_column($plugin, $table, $data) { + return true; + } +} + +if (!function_exists('api_plugin_db_table_create')) { + function api_plugin_db_table_create($plugin, $table, $data) { + return true; + } +} + +if (!function_exists('read_config_option')) { + function read_config_option($name, $force = false) { + return ''; + } +} + +if (!function_exists('set_config_option')) { + function set_config_option($name, $value) { + } +} + +if (!function_exists('html_escape')) { + function html_escape($string) { + return htmlspecialchars($string, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + } +} + +if (!function_exists('__')) { + function __($text, $domain = '') { + return $text; + } +} + +if (!function_exists('__esc')) { + function __esc($text, $domain = '') { + return htmlspecialchars($text, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + } +} + +if (!function_exists('cacti_log')) { + function cacti_log($message, $also_print = false, $log_type = '', $level = 0) { + } +} + +if (!function_exists('cacti_sizeof')) { + function cacti_sizeof($array) { + return is_array($array) ? count($array) : 0; + } +} + +if (!function_exists('is_realm_allowed')) { + function is_realm_allowed($realm) { + return true; + } +} + +if (!function_exists('raise_message')) { + function raise_message($id, $text = '', $level = 0) { + } +} + +if (!function_exists('get_request_var')) { + function get_request_var($name) { + return ''; + } +} + +if (!function_exists('get_nfilter_request_var')) { + function get_nfilter_request_var($name) { + return ''; + } +} + +if (!function_exists('get_filter_request_var')) { + function get_filter_request_var($name) { + return ''; + } +} + +if (!function_exists('form_input_validate')) { + function form_input_validate($value, $name, $regex, $optional, $error) { + return $value; + } +} + +if (!function_exists('is_error_message')) { + function is_error_message() { + return false; + } +} + +if (!function_exists('sql_save')) { + function sql_save($array, $table, $key = 'id') { + return isset($array['id']) ? $array['id'] : 1; + } +} + +if (!defined('CACTI_PATH_BASE')) { + define('CACTI_PATH_BASE', '/var/www/html/cacti'); +} + +if (!defined('POLLER_VERBOSITY_LOW')) { + define('POLLER_VERBOSITY_LOW', 2); +} + +if (!defined('POLLER_VERBOSITY_MEDIUM')) { + define('POLLER_VERBOSITY_MEDIUM', 3); +} + +if (!defined('POLLER_VERBOSITY_DEBUG')) { + define('POLLER_VERBOSITY_DEBUG', 5); +} + +if (!defined('POLLER_VERBOSITY_NONE')) { + define('POLLER_VERBOSITY_NONE', 6); +} + +if (!defined('MESSAGE_LEVEL_ERROR')) { + define('MESSAGE_LEVEL_ERROR', 1); +}