We attempted to fix this twice already, so this change reverses r36709
(which simply ignored all Time Exceeded messages for host discovery,
leading to long scan times when the TTL is insufficient) and r34557
(which used incorrect logic to match ICMP responses, resulting in
ignoring all ICMP error messages in response to ICMP probes, e.g. Time
Exceeded for Echo Request).
Instead, we'll correctly verify that the ICMP ID of the *encapsulated
datagram* matches the one for the probe we are attempting to match.
http://seclists.org/nmap-dev/2015/q2/68
From: David Fifield <david () bamsoftware com>
Date: Wed, 29 Apr 2015 10:13:40 -0700
Previously, a Time Exceeded message meant the host was immediately
marked down, even if there were other probes still in transit. Now it
gets treated the same as no-response.
I found that a concurrent ICMP traceroute was interfering with host
discovery. Nmap was receiving and acting on the Time Exceeded messages
produced by the traceroute process. This caused it to mark a host as
down that actually was up.
Two issues here: First, IP protocol scan can send packets with protocol
58 (ICMPv6) even over IPv4. This led to a bad interaction where the
packet was created (in build_protoscan_packet) without a data payload,
but setIP tried to set the packet's Identifier field (present in both
ICMPv6 and ICMP Echo Request packets), leading to a heap buffer
overflow. Instead, we now only try to set this identifier when the IP
version matches the ICMP version, indicating that we set the data
payload.
The other issue was a out-of-bounds read while packet tracing when an
ICMPv6 packet without a payload was sent or received, due to trying to
read the type and code. Now we check that the data length is sufficient
to contain an ICMPv6 header before attempting to read one.
Credit LLVM/Clang's AddressSanitizer with catching these bugs.