onesixtyone icon indicating copy to clipboard operation
onesixtyone copied to clipboard

Fix destination port changes unexpectedly during scan

Open f10d0 opened this issue 9 months ago • 1 comments

There is a bug where whenever the scanner receives a reply from another port than the set destination SNMP port (161) all subsequently send probe packets will be send to this port.

This is because send and receive use the same sockaddr_in struct (remote_addr). Incoming replies from a rogue target device can alter the destination port in the next iteration and therefore make the scanner stop working correctly and send unsolicited packets.

I fixed this by setting the destination port before each send iteration together with the next destination address.

f10d0 avatar Mar 12 '25 14:03 f10d0

PR Review: ✅ MERGE RECOMMENDED

This PR fixes a bug that can cause scanner failure. Excellent catch, @f10d0!

The Bug

The scanner reuses the same remote_addr struct for both sending (sendto) and receiving (recvfrom). When recvfrom() is called at line 875, it overwrites the entire struct with the source address of the response - including the source port. If any device responds from a non-standard port (not 161), all subsequent probes get sent to the wrong port, breaking the scan for all remaining hosts.

The Fix

Moving remote_addr.sin_port = htons(o.port); inside the loop ensures the correct destination port is always set before each sendto(). This is the correct fix.

Why This Should Be Merged

  1. Real Bug: This isn't theoretical - any device responding from a non-standard source port will corrupt the scanner's destination port for all subsequent hosts
  2. Correct Fix: The solution properly addresses the root cause without side effects
  3. Minimal Change: Just 1 line moved - perfectly matches onesixtyone's tight coding style
  4. Zero Performance Impact: htons() is typically a macro doing bit-shifting; adds nanoseconds to operations taking milliseconds
  5. No Breaking Changes: The change is isolated and maintains all existing behavior

Code Quality

The fix exemplifies onesixtyone's philosophy:

  • Extremely minimal (1 line moved)
  • No new variables or complexity
  • Clear and obvious what it does
  • Follows existing patterns

This is a textbook example of a good bug fix - minimal, correct, and essential.

cc @alexsotirov for merge approval

dguido avatar Aug 30 '25 17:08 dguido