Re: IP cheksum and libnet_pblock_coalesce

Frédéric Raynal <[email protected]> Sun, 26 Sep 2004 11:40:03 +0200
Newsgroups gmane.comp.security.libnet
Message-ID <[email protected]>
Hi,

That is a known (but tricky) bug reported some times ago.
David Baroso proposed something like :

sum += ((*(u_int8_t *)addr & 0xFF)<<8);

The difficulty is to find something working on both big and little
endian systems. 

	Fred Raynal

  

On Fri, Sep 24, 2004 at 03:27:56PM +0300, Jari Lansio wrote:
> 
> 
> Hello,
> 
> I think my problem _was_ just a misunderstanding from my part. The 
> pressure is on word "was", because now I think I have found a genuine 
> bug in checksum code.
> 
> This is function libnet_in_cksum() from libnet_checksum.c:
> 
>     41   int
>     42   libnet_in_cksum(u_int16_t *addr, int len)
>     43   {
>     44       int sum;
>     45
>     46       sum = 0;
>     47
>     48       while (len > 1)
>     49       {
>     50           sum += *addr++;
>     51           len -= 2;
>     52       }
>     53       if (len == 1)
>     54       {
>     55           sum += *(u_int16_t *)addr;
>     56       }
>     57
>     58       return (sum);
>     59   }
> 
> Since the type of argument 'addr' is pointer to u_int16_t, it would seem 
> like this is function is supposed to work only with memory blocks that 
> have even length.
> 
> However, the code at line 53 checks if buffer has odd length. This is 
> important, because this function is used to calculate checksum for UDP 
> and TCP too, and they can have odd lengths.
> 
> I think that there is bug in on line 55, where 'addr' is casted to 
> u_int16_t*, the type it already is!!! The problem is that this code 
> reads 2 bytes from address 'addr', while it should read only one byte.
> 
> I changed to line 55 to read like this:
> 
>     55           sum += *(u_int8_t *)addr;
> 
> Now only one byte is read (as the 'len' indicates) and everything seems 
> to work just fine.
> 
> How this bug has not caused any problems before? The only explanation is 
> that the compilers add 'padding' bytes right after buffers with odd 
> length, and that padding is always '0', and thus including this padding 
> to checksum calculation do not change the checksum. However, I used this 
> code on Symbian platform and guess what, there was no padding and it 
> failed to get correct checksums for packets that had odd length.
> 
> The rfc-1071 has example C code for checksum calculation, and it looks 
> very similar. It's not perfect example, since it has a bug that prevents 
> it to compile (missing '*' on line 381), but the relevant lines are:
> 
>    385              /*  Add left-over byte, if any */
>    386          if( count > 0 )
>    387                  sum += * (unsigned char *) addr;
> 
> Notice the cast to u_int8_t.
> 
> Am I right? Have I found a real bug, or did I just made myself look 
> stupid :) I'm on holidays the next week and I won't read my mails until 
> 4th of October.
> 
> -jarppe
> 
> > Hi jarppe
> >
> >> Hello everybody,
> >>
> >> Should libnet_pblock_coalesce() set the IP checksum correctly? I'm 
> using libnet 1.1.2.1, compiled with GCC 3.2.2 20030222 on Linux.
> >>
> >> When I create packet using libnet_build_[udp|tcp]() + 
> libnet_build_ip(), then get the packet using libnet_pblock_coalesce() 
> and then call libnet_ip_check() over the returned buffer, I get non-zero 
> reply, indicating wrong IP checksum.
> >>
> >> Interestingly enough, if I create the packet with 
> libnet_build_icmpv4_echo() + libnet_build_ip(), then libnet_ip_check() 
> returns zero (checksum ok).
> >>
> >> Am I doing something wrong, or is there a bug?
> >>
> >> -jarppe
> >
> >
> > I had something similar situation. I'm also using libnet 1.1.2.1 with
> > GCC (Redhat9).
> >
> > I also found some wrong checksum packets created by libnet.
> > So, I replace the function libnet_in_checksum() (libnet_checksum.c) with
> > the same name function of libnet-1.0.2a, there is no wrong packet.
> >
> > -Kenichi
> >
>