Wireshark · Ethereal-dev: Re: [Ethereal-dev] Re: rev 17527: /trunk/plugins/profinet/: packet-pn-dcp.c

[Ulf Lamping <ulf.lamping@xxxxxx>]

Lars Roland wrote:
> I'm not familiar with this protocol, too.
I'm a bit more familiar with this protocol as I've written the 
dissector, so I'm the one to blame :-)
> However I guess that it is mandatory in this protocol to have a length 
> indicator after the type and subtype indicator.
> Thus the length of the value should be stored unconditionally in 
> "block_length", like in dissect_PNDCP_Suboption_IP().
I'm not sure if that's really the case under all circumstances (just 
can't remember, writing the dissector is already some time ago).
> If the remaining length of the pdu, which is stored in "length", is 
> big enough to contain the whole tlv block, should be checked seperately.
>
> IMO in its current state the dissector is not very robust.
Ooops ...

I've done some fuzz-testing on this dissector so it shouldn't be too bad.
> I have some time left to work on these issues on the weekend, but I 
> don't have any traces to test with or a specification to verify my 
> assumptions on the protocol structure.
Unfortunately, I can't hand you the specs or traces :-(
> As a short time workaround, it should be sufficient to throw an 
> exception, if "block_length" is used uninitialized.
The DCP packets are usually short, so there shouldn't be problems with 
fragmenting or such. So if block_length is used uninitialized it's worth 
an exception and that might even be the "long time workaround" :-)

Or simply initialize block_length with 0 as that would be also ok.

If someone can pass me the coverity reports of this dissector, I'll have 
a look.

Regards, ULFL