["Lars Roland" <Lars.Roland@xxxxxxx>]

Use this updated patch instead.

Am 10.03.2006, 16:20 Uhr, schrieb Lars Roland <Lars.Roland@xxxxxxx>:

> Am 10.03.2006, 09:47 Uhr, schrieb 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).
>
> It makes only sense to have no length field, if the value consumes all
> remaining bytes of the pdu.
>
> > > 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.
>
> So practical tests invalidate my theory. It is probably more robust than  
> I
> thought. :)
>
> > > 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.
>
> Coverity just complains that block_length might be used uninitialized in
> some weird conditions.
> IMO it might be possible (I'm not sure about that) to get into an  
> infinite
> loop, as this value can influence the stop condition of a while loop.
>
> The attached patch will solve coverity's and my complains. If these
> changes are OK for you, I'll check them in.
>
> Best regards,
> Lars

Attachment: packet-pn-dcp.c.diff
Description: Binary data