Wireshark · Ethereal-dev: Re: [Ethereal-dev] Patch to packet-bacapp.c

[Guy Harris <gharris@xxxxxxxxx>]

Guy Harris wrote:

> Checked in.

...and backed out, because its heavy use of recursion instead of 
iteration means that malformed packets can cause it to overflow the 
process's stack and crash.

Please fix that, and also:

	1) fix the problems noted in Ronnie Sahlberg's and my mail messages;

	2) don't use "match_strval()" to convert numerical values to strings, 
use "val_to_str()", as a malformed packet could mean that there is no 
string corresponding to the value;

	3) don't just cast length values to "guint8" if they might be larger 
than 8 bits;

	4) make the length field in the tag/class/LTV value 32 bits, as the 
BACnet spec I have seems to say it can't be larger than 2^32-1 - and 
make the tag parsing code handle the "length is > 65535" case;

	5) don't use "//" comments - not all compilers Ethereal is built with 
support them;

	6) make all routines not used outside packet-bacapp.c static.

You might want to consider modeling the BACnet application layer 
dissector after the packet-ber.c dissector and dissectors using it, e.g. 
with SEQUENCE, SEQUENCE OF, and CHOICE values dissected with a table 
handed to a generic SEQUENCE, SEQUENCE OF, or CHOICE dissector.