Wireshark-dev: Re: [Wireshark-dev] Bug 976
From: Martin Mathieson <[email protected]>
Date: Fri, 21 Jul 2006 10:28:29 +0100
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.