Wireshark · Ethereal-dev: [Ethereal-dev] Re: ethereal 0.10.8 radius/iapp dissector vuln

[Guy Harris <gharris@xxxxxxxxx>]

Jonathan Heusser wrote:
> > If the fourth argument to "rdconvertbufftostr()" is negative, there's 
> > a bug in whatever code is calling it.
>
> Depends where you're normally fixing bugs..

I try to fix them where they're located, rather than where they 
immediately manifest themselves, as that makes it more likely that, for 
example, the real problem can be shown to the user.

> The attached file is an example packet which let ethereal crash, 
> (ab)using the tagged string case.

The current version in SVN doesn't crash, as one of the explicit AVP 
length checks I added makes sure you have at least one byte present for 
the tag in a tagged string - although it also doesn't report that 
there's a bad tagged string, so I'll have to see why that is.

> > > A simple fix would be to bail out when 'length' is negative.
> >
> > "Bailing out" should be done with [..]
>
> As I said, a simple fix.

In the current SVN code, the bailing out *is* done by calling 
"g_assert()", so that Ethereal will crash if a negative length is passed 
- as per the above, if that happens, there's a bug somewhere and it has 
to be fixed there.