Re: Bug ? [was: Re: icmp checksum with build_data()s]

sandr8 <[email protected]> Mon, 24 May 2004 13:52:28 +0200
Newsgroups gmane.comp.security.libnet
Message-ID <[email protected]>
Yep,
 not only if  the second build_foo() has a different size, i guess it 
would happen also if it stays the same. This because the next time 
l->total_size will also take into account the size of "bar".
The solution would be to add a field into any pblock structure and to 
update it in way pretty similar to the one used to update 
l->total_size.  If you agree that the library should behave in this way 
(taking into account any payload and not only those specified inline by 
u_char *payload, int payload_s) i can try to write a patch asap (that is 
after wednesday, i'm quite busy at the moment :).
cheers
thank you.
sandr8)

Frederic Raynal wrote:

> Re,
> The (geek) party is going on ... and the techno music made me think of 
> a potential bug in all builders using the "solution" I proposed bellow.
> Example:
> ptag = libnet_build_foo(); // <-- dont use l->total_size
> libnet_build_bar(); // <-- use l->total_size
> libnet_write(); // <-- Ok
> libnet_build_foo(..., ptag); // <--  BUG CAUSED HERE
> ...
> If the second build_foo() has a different size, then there will be an 
> error in checksum computation as total_size will have changed :-(
> This need to be checked ... but it's too late for me now.
>
> Fred Raynal
> Frédéric Raynal writes:
>
>> Hi,
>> I am currently at Berlin (others there ?)
>> As far as I can tell, you sould not use it that way: the checksums are
>> computed according to what is in the pblock, not after. 
>> But you are right here: this problem is also present for all other
>> packets that do include paylaod in the checksum computation :(
>> raynal@Joker:~/LIBNET/libnet/src$ grep "h =" *c|grep payload
>> libnet_build_cdp.c:    h = LIBNET_CDP_H + len + payload_s;
>> libnet_build_icmp.c:    h = LIBNET_ICMPV4_ECHO_H + payload_s; /* hl 
>> for checksum */
>> libnet_build_icmp.c:    h = LIBNET_ICMPV4_MASK_H + payload_s; /* hl 
>> for checksum */
>> libnet_build_icmp.c:    h = LIBNET_ICMPV4_TS_H + payload_s;        /* 
>> hl for checksum */
>> libnet_build_icmp.c:    h = LIBNET_ICMPV4_UNREACH_H + payload_s + 
>> l->total_size; libnet_build_icmp.c:    h = LIBNET_ICMPV4_TIMXCEED_H + 
>> payload_s + l->total_size; libnet_build_icmp.c:    h = 
>> LIBNET_ICMPV4_REDIRECT_H + payload_s + l->total_size; 
>> libnet_build_ospf.c:    h = LIBNET_OSPF_H + payload_s + len;
>> libnet_build_ospf.c:    h = len + payload_s;
>> libnet_build_vrrp.c:    h = LIBNET_VRRP_H + payload_s;
>>
>> Without any garantee, the solution might be to systematically include
>> "+ l->total_size;" so that the data is included each time it is
>> required for the checksum.
>>
>>     Fred Raynal
>