Suspect code in libnet_build_ip.c:libnet_build_ipv4()

[ThomasHabets]

Line 188 has this loop:

for (p_temp = l->protocol_blocks;
     p_temp->type == LIBNET_PBLOCK_IPV4_H ||
     p_temp->type == LIBNET_PBLOCK_IPO_H;
     p_temp = p_temp->next)
{
    libnet_pblock_insert_before(l, p_temp->ptag, p_data->ptag);
    break;
}

I don't know exactly what this code is trying to do, but it's not doing it.

p_temp = p_temp->next will never be executed, due to the break.

The comment a few lines up is not comforting either:

/* SR - I'm not sure how to reach this code. Maybe if the first                                                                               
 * time we added an ipv4 block, there was no payload, but when                                                                                
 * we modify the block the next time, we have payload?                                                                                        
 */

[troglobit]

Hmm, yeah that doesn't really look good ... maybe @sam-github can provide some insight?

[sam-github]

the code's probably just wrong and needs to be fixed. hopefully git blame doesn't point at me :-)

I haven't looked at this stuff in years, so I've nothing specific to add. The pblock linking is subtle, and segvs when it is wrong, which is what originally got me into maintaining libnet. While reading and debugging I did leave comments around through the code, some not very flattering. That looks like one. Beware that some of the existing comments were wrong or misleading, and I don't guarantee that I fixed them all.

From memory the pblocks are part of a (doubly?) linked list... but, pblock coalescing is "type" aware... that is, IPv4 lengths will be pulled from the public data to figure out how to link to the next pblock, and perhaps index into it. It mostly goes OK, but if you use libnet to generate invalid packets you can easily confuse it and segv or overwrite memory. Its better at crafting valid packet nesting, maybe with the innermost packet being data pblock type, and that can then contain nasty invalid packets.