Wireshark-dev: Re: [Wireshark-dev] [PATCH] New Dissector : Roofnet
From: Sebastien Tandel <sebastien@xxxxxxxxx>
Date: Tue, 19 Dec 2006 19:31:50 +0100
>>> * The loop in dissect_roofnet should check that it doesn't spin out of
>>> control when an incorrectly large value is read.
>>>
>>>       
>> The roofnet length is restricted to 400 bytes (maybe 200 in a near
>> future). I then control whetherr the length of the announced number of
>> links is greater than this max length (400). If it's the case I print an
>> error in the tree, add an expert info value and stop the dissection of
>> the packet. Is it sufficient?
>>     
>
> I rather would like a check against the actual size of the packet, to
> avoid going out of bounds at tvb access.
>   
ok done
I also let the other check which in fact will decode the header first
... then do the check, add the expert info and stop the dissection of
this packet.

>>> * Use the 'standard' file header as found in the README.developer
>>>
>>>       
>> Did you mean stdio, stdlib ? If not, give me a hint 'cause I don't see ...
>>     
>
> I mean the copyright stuff, like
>   
Ah! ok :) ... it was there but without the mention to the original
author Gerald Combs. Sorry! :-p

> Furhter question: did you fuzz test this dissector on some real life
> roofnet captures?
>   
On the modified version? ... At the time of writing your question ...
no. Now, yes! :)
525 passes on 7250 frames for each pass (with the modifications I did now).
Is it sufficient? Or have you stronger requisites?


Regards,
Sebastien Tandel