Re: Nmap/Nping bug on udp checksum calculation

[Daniel Miller <bonsaiviking () gmail com>]

On 11/02/2013 05:38 PM, Michael Weber wrote:
> Sending UDP data can contain but must not contain a checksum. The checksum,
> must be transmitted as all ones if the calculated value is all zero. An all
> zero value denotes that no checksum has been calculated (see rfc 768)
> In Nmap the checksum is calculated in netutil.cc (ipv4_pseudoheader_cksum)
> called from within UDPHeader.cc (UDPHeader::setSum) as a quick search in the
> sources revealed.
> I think the return value must be checked for zero like it's done in
> ipv6_pseudoheader_cksum before it is returned or, if that will lead to some
> trouble for TCP  checksum calculation, must be checked in UDPHeader::setSum
> before the method returns.
Michael,

Thanks for catching this! I wrote some crappy test code to check how 
often this would occur. Given an empty UDP packet from my machine to 
scanme.nmap.org with random source and destination ports, a 0x0000 
checksum was generated 0.001527% of the time (1 in 65488 chance, so I'm 
guessing overall it's closer to 1:65535).

The patch below should fix this. I did a check for everywhere in the 
Nmap source that a UDP header's uh_sum attribute was set, and they all 
stemmed from ipv6_pseudoheader_cksum and ipv4_pseudoheader_cksum, so I 
just copied the check code over. I also moved the call to 
ip_cksum_carry() to before the check in both cases, since that alters 
the value before the check for +/-0 should be done.

Please check the code below and let me know if it looks safe to commit. 
I'm looking forward to killing this bug!

Dan

diff --git a/libnetutil/netutil.cc b/libnetutil/netutil.cc
index 482d111..28c624c 100644
--- a/libnetutil/netutil.cc
+++ b/libnetutil/netutil.cc
@@ -844,7 +844,15 @@ unsigned short ipv4_pseudoheader_cksum(const struct 
in_addr *src,
   sum = ip_cksum_add(hstart, len, sum);

   /* Fold in the carry, take the complement, and return. */
-  return ip_cksum_carry(sum);
+  sum = ip_cksum_carry(sum);
+  /* RFC 768: "If the computed  checksum  is zero,  it is transmitted  
as all
+   * ones (the equivalent  in one's complement  arithmetic).   An all zero
+   * transmitted checksum  value means that the transmitter generated  no
+   * checksum" */
+  if (proto == IP_PROTO_UDP && sum == 0)
+    sum = 0xFFFF;
+
+  return sum;
 }

 /* Calculate the Internet checksum of some given data concatenated 
with the
@@ -868,6 +876,7 @@ u16 ipv6_pseudoheader_cksum(const struct in6_addr *src,

   sum = ip_cksum_add(&hdr, sizeof(hdr), 0);
   sum = ip_cksum_add(hstart, len, sum);
+  sum = ip_cksum_carry(sum);
   /* RFC 2460: "Unlike IPv4, when UDP packets are originated by an 
IPv6 node,
      the UDP checksum is not optional.  That is, whenever originating 
a UDP
      packet, an IPv6 node must compute a UDP checksum over the packet 
and the
@@ -876,7 +885,7 @@ u16 ipv6_pseudoheader_cksum(const struct in6_addr *src,
   if (nxt == IP_PROTO_UDP && sum == 0)
     sum = 0xFFFF;

-  return ip_cksum_carry(sum);
+  return sum;
 }

 void sethdrinclude(int sd) {

_______________________________________________
Sent through the dev mailing list
http://nmap.org/mailman/listinfo/dev
Archived at http://seclists.org/nmap-dev/