Wireshark · Wireshark-dev: Re: [Wireshark-dev] Bug 976

[Martin Mathieson <martin.mathieson@xxxxxxxxxxxx>]

Martin Mathieson wrote:

> Guy Harris wrote:
>
>  
>
> > On Jun 27, 2006, at 5:51 AM, Martin Mathieson wrote:
> >
> >
> >
> >    
> >
> > > Looking at frame 170 in the trace, it looks like  
> > > tvb_get_ephemeral_text() struggles with the null character in the  
> > > middle of the 4th parameter (in the WWW-Authenticate header) and  
> > > returns NULL.
> > >   
> > >
> > >      
> > That shouldn't happen - tvb_get_ephemeral_string() (not _text) just  
> > uses memcpy() to copy the string, so it shouldn't be affected by a  
> > null character.  It can only return null if ep_alloc() returns null -  
> > and if null pointers cause a problem, you'll crash in
> >
> > 	strbuf[length] = '\0';
> >
> > before tvb_get_ephemeral_string() returns.
> >
> > _______________________________________________
> >
> >
> >    
> It looks like I was jumping to conclusions, I don't think it has to do 
> with the null character within the string.
>
> The fuzz testing generated a %s inside the parameter name.  This means 
> that its looking for a string arg that doesn't exist.  Which means that 
> although the patch that's been applied makes the string display better, 
> it doesn't fix the bug for implementations of vsnprintf() that can't 
> cope with missing args for specifiers.
>
> proto_tree_add_text() is called from thousands of places - are they too 
> vulnerable to crashing when they meet what happen to look like 
> specifiers (with vsnprintf() implementations the one that first found 
> this bug) ?
>
>  
I hadn't realised that gerald  fixed this (r18600) by making the format 
string "%s". And of course every other call to proto_tree_add_text() 
I've seen uses a literal format string.