Skip to content

fix(mactrack): fix 9 logic bugs in MAC address formatting, DNS resolver, and port scanning#333

Open
somethingwithproof wants to merge 3 commits into
Cacti:developfrom
somethingwithproof:fix/mactrack-logic-bugs
Open

fix(mactrack): fix 9 logic bugs in MAC address formatting, DNS resolver, and port scanning#333
somethingwithproof wants to merge 3 commits into
Cacti:developfrom
somethingwithproof:fix/mactrack-logic-bugs

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

Summary

  • Bug 1 [CRITICAL]: xform_mac_address() read $max_address (typo) instead of $mac_address — all MAC addresses returned as empty string or raw input
  • Bug 3 [CRITICAL]: poller_mactrack.php UPDATE concatenated device_id . mac_address into one parameter — mac_track_temp_ports IP updates never matched any row
  • Bug 2 [HIGH]: mactrack_resolver.php DNS resolver initialized only when $nameservers is empty; branches swapped — configured DNS servers were always ignored
  • Bug 5 [HIGH]: mactrack_devices.php ignorePorts bulk UPDATE used WHERE id = ? instead of WHERE device_id = ? — batch updates silently wrote zero rows (confirmed: all other queries in same file use device_id)
  • Bug 6 [HIGH]: macauth email timing used > time() instead of < time() — report fired continuously before scheduled time, never after
  • Bug 7 [MEDIUM]: $active_vlans++ incremented an array as scalar in 6 vendor files (hp, hp_ng, hp_ngi, extreme, juniper, foundry) — clobbered VLAN array, port scanning never executed
  • Bug 9 [MEDIUM]: strpos($parameter, '=') used as boolean without !== false in 3 files — parameters starting with = misclassified
  • Bug 10 [MEDIUM]: mactrack_hp_ng.php ifName lookup used $ifInterfaces['ifAlias'][$ifIndex] instead of $ifInterfaces[$ifIndex]['ifAlias'] — all port names always empty

Note: ARP MIB condition ($ifIntcount != 0 selecting ipNetToMedia OIDs) was reviewed and confirmed correct — device responsiveness to AT-MIB probe gates use of the newer ipNetToMedia table.

Test plan

  • Verify MAC addresses are correctly formatted after xform_mac_address()
  • Verify mac_track_temp_ports IP column is updated during ARP resolution
  • Verify DNS resolution uses configured nameservers when set
  • Verify ignorePorts batch update writes correctly to mac_track_devices
  • Verify VLAN port scanning executes for HP/Extreme/Juniper/Foundry devices
  • Verify port names populated in hp_ng collector

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings May 17, 2026 09:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a collection of independent logic bugs across the mactrack plugin: a typo'd variable in MAC address normalization, an argument-count mismatch in a prepared UPDATE, inverted/swapped DNS resolver initialization, wrong WHERE column in a bulk device update, an inverted comparison in the macauth email scheduler, scalar-increment misuse on array variables in six vendor scan modules, missing strict !== false checks on strpos(), and a reversed array index in the hp_ng ifAlias lookup.

Changes:

  • Correct variable references and comparison/condition operators in MAC formatting, ARP UPDATE, DNS resolver branch, macauth timing, ignorePorts bulk update, and hp_ng ifAlias lookup.
  • Remove erroneous $active_vlans++ scalar-increment on array variables across HP/Extreme/Juniper/Foundry collectors.
  • Add !== false to strpos($parameter, '=') checks in the three CLI entry scripts.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file
File Description
poller_mactrack.php Fix strpos truthiness check, split concatenated device_id/mac_address parameter, invert macauth scheduling comparison
mactrack_scanner.php Add !== false to strpos arg parser check
mactrack_resolver.php Add !== false to strpos check; swap branches so Net_DNS2_Resolver is initialized when nameservers are configured
mactrack_devices.php Change ignorePorts UPDATE WHERE clause from id to device_id
lib/mactrack_functions.php Use correct $mac_address instead of typo $max_address in xform_mac_address()
lib/mactrack_hp.php, hp_ng.php, hp_ngi.php, extreme.php, juniper.php, foundry.php Remove invalid $active_vlans++ increment on array variable
lib/mactrack_hp_ng.php Correct ifAlias lookup to $ifInterfaces[$ifIndex]['ifAlias']

…rt scanning

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Confirmed != 0 is correct: when device responds to AT-MIB probe
(ifIntcount > 0), use newer ipNetToMedia OIDs for collection.

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof force-pushed the fix/mactrack-logic-bugs branch from e4d876b to 961ea1c Compare May 17, 2026 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants