From 6a85cc714d2cc01e8261ecba77ba6980fe56a356 Mon Sep 17 00:00:00 2001 From: Thomas Vincent Date: Mon, 31 Aug 2026 02:12:43 -0700 Subject: [PATCH] fix(mactrack): correct xform_mac_address typo and resolver/VLAN logic bugs Signed-off-by: Thomas Vincent --- lib/mactrack_enterasys.php | 1 - lib/mactrack_extreme.php | 1 - lib/mactrack_foundry.php | 1 - lib/mactrack_functions.php | 18 ++++--- lib/mactrack_hp.php | 1 - lib/mactrack_hp_ng.php | 3 +- lib/mactrack_hp_ngi.php | 1 - lib/mactrack_juniper.php | 1 - lib/mactrack_norbay.php | 2 - lib/mactrack_norbay_ng.php | 1 - mactrack_devices.php | 2 +- mactrack_resolver.php | 8 +-- mactrack_scanner.php | 2 +- poller_mactrack.php | 15 ++++-- tests/Pest/Unit/XformMacAddressTest.php | 65 +++++++++++++++++++++++++ 15 files changed, 95 insertions(+), 27 deletions(-) create mode 100644 tests/Pest/Unit/XformMacAddressTest.php diff --git a/lib/mactrack_enterasys.php b/lib/mactrack_enterasys.php index 3b51c7ba..cdfc1ec7 100644 --- a/lib/mactrack_enterasys.php +++ b/lib/mactrack_enterasys.php @@ -100,7 +100,6 @@ function get_enterasys_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_extreme.php b/lib/mactrack_extreme.php index 4682983e..6d24094b 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 3440e14e..37bc05fe 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 030d045f..379e6f7c 100644 --- a/lib/mactrack_functions.php +++ b/lib/mactrack_functions.php @@ -2127,17 +2127,21 @@ function xform_net_address($ip_address) { * @param mixed $mac_address */ function xform_mac_address($mac_address) { - $max_address = trim($mac_address); + $mac_address = trim((string) $mac_address); - if ($mac_address == '') { - $mac_address = 'NOT USER'; - } elseif (strlen($mac_address) > 10) { // return is in ascii - $max_address = str_replace( + if ($mac_address === '') { + return 'NOT USER'; + } + + if (strlen($mac_address) > 10) { + // ASCII / HEX- form (e.g. "HEX-00:aa:bb:...", "aa-bb-cc-dd-ee-ff") + $mac_address = str_replace( ['HEX-00:', 'HEX-:', 'HEX-', '"', ' ', '-'], ['', '', '', '', ':', ':'], $mac_address ); - } else { // return is hex + } else { + // Binary hex bytes from SNMP $mac = ''; for ($j = 0; $j < strlen($mac_address); $j++) { @@ -2147,7 +2151,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); } diff --git a/lib/mactrack_hp.php b/lib/mactrack_hp.php index da9d18e4..036c33d4 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 1bf274bc..6bca5b1a 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 d4a3309b..8a22f742 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 389f56a5..5bf14ad3 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/lib/mactrack_norbay.php b/lib/mactrack_norbay.php index b739c00c..c0b8c2ff 100644 --- a/lib/mactrack_norbay.php +++ b/lib/mactrack_norbay.php @@ -80,7 +80,6 @@ function get_norbay_accelar_switch_ports($site, &$device, $lowPort = 0, $highPor 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++; } } @@ -199,7 +198,6 @@ function get_norbay_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_norbay_ng.php b/lib/mactrack_norbay_ng.php index 2b6095f6..ea6cc93b 100644 --- a/lib/mactrack_norbay_ng.php +++ b/lib/mactrack_norbay_ng.php @@ -72,7 +72,6 @@ function get_norbay_ng_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/mactrack_devices.php b/mactrack_devices.php index cb479b28..910c8914 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 83ca4f1e..ed344219 100644 --- a/mactrack_resolver.php +++ b/mactrack_resolver.php @@ -85,7 +85,7 @@ if (cacti_sizeof($parms)) { foreach ($parms as $parameter) { - if (strpos($parameter, '=')) { + if (strpos($parameter, '=') !== false) { [$arg, $value] = explode('=', $parameter); } else { $arg = $parameter; @@ -158,11 +158,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 4b03f442..e59af538 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 dcc8c86c..987f12ae 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; @@ -729,7 +729,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']]); } } } @@ -1007,7 +1007,10 @@ 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()) || + $last_macauth_time = (int) $last_macauth_time; + $mac_auth_frequency = (int) $mac_auth_frequency; + + if (($last_macauth_time + ($mac_auth_frequency * 60) < time()) || ($mac_auth_frequency == 0)) { mactrack_process_mac_auth_report($mac_auth_frequency, $last_macauth_time); } @@ -1116,6 +1119,11 @@ function mactrack_process_mac_auth_report($mac_auth_frequency, $last_macauth_tim // email the report mactrack_mail($to, $from, $fromname, $subject, $message, $headers = ''); mactrack_debug('MACAUTH Report eMail sent.'); + + // Persist last-run so the schedule gate does not re-fire every poll. + if ($mac_auth_frequency > 0) { + set_config_option('mt_last_macauth_time', (string) time()); + } } else { // email the report @@ -1130,6 +1138,7 @@ function mactrack_process_mac_auth_report($mac_auth_frequency, $last_macauth_tim // email the report mactrack_mail($to, $from, $fromname, $subject, $message, $headers = ''); mactrack_debug('MACAUTH Report empty eMail sent.'); + set_config_option('mt_last_macauth_time', (string) time()); } } } diff --git a/tests/Pest/Unit/XformMacAddressTest.php b/tests/Pest/Unit/XformMacAddressTest.php new file mode 100644 index 00000000..d52bb563 --- /dev/null +++ b/tests/Pest/Unit/XformMacAddressTest.php @@ -0,0 +1,65 @@ + 10) { + $mac_address = str_replace( + ['HEX-00:', 'HEX-:', 'HEX-', '"', ' ', '-'], + ['', '', '', '', ':', ':'], + $mac_address + ); + } else { + $mac = ''; + + for ($j = 0; $j < strlen($mac_address); $j++) { + $mac .= bin2hex($mac_address[$j]) . ':'; + } + + $mac_address = $mac; + } + + $mac_address = str_replace(':', '', $mac_address); + + return strtoupper($mac_address); +} + +test('empty MAC becomes NOT USER', function () { + expect(test_xform_mac_address(''))->toBe('NOT USER'); + expect(test_xform_mac_address(' '))->toBe('NOT USER'); +}); + +test('ASCII and HEX- forms strip delimiters', function () { + expect(test_xform_mac_address('aa-bb-cc-dd-ee-ff'))->toBe('AABBCCDDEEFF'); + expect(test_xform_mac_address('HEX-00:aa:bb:cc:dd:ee:ff'))->toBe('AABBCCDDEEFF'); + expect(test_xform_mac_address('aa:bb:cc:dd:ee:ff'))->toBe('AABBCCDDEEFF'); +}); + +test('binary hex bytes convert to uppercase hex string', function () { + $binary = hex2bin('aabbccddeeff'); + expect(test_xform_mac_address($binary))->toBe('AABBCCDDEEFF'); +}); + +test('production xform_mac_address keeps a single working variable', function () { + $src = file_get_contents(dirname(__DIR__, 3) . '/lib/mactrack_functions.php'); + // The max_address typo path must not reappear. + expect($src)->not->toContain("str_replace(':', '', \$max_address)"); + expect($src)->toContain("str_replace(':', '', \$mac_address)"); + expect($src)->toContain("return 'NOT USER'"); +}); + +test('macauth schedule persists last-run time', function () { + $src = file_get_contents(dirname(__DIR__, 3) . '/poller_mactrack.php'); + expect($src)->toContain("set_config_option('mt_last_macauth_time'"); + expect($src)->toContain('$last_macauth_time + ($mac_auth_frequency * 60) < time()'); +});