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 >