We're now a non-profit! Support open source packet analysis by making a donation.

Wireshark-dev: Re: [Wireshark-dev] strlen() and NULL pointer checks

From: Jeff Morriss <[email protected]>
Date: Mon, 16 May 2011 17:53:11 -0400
Jakub Zawadzki wrote:
On Mon, May 16, 2011 at 04:18:40PM -0400, Jeff Morriss wrote:
Every once in a while, I do some fuzz testing on a Solaris/SPARC system. When I first did it I was primarily worried about getting bus errors (due to casts increasing alignment requirements), but usually what I find is another case of what I fixed in r37181. (Fortunately, I have not gotten bus errors.)
The backtrace for that one was:

#0  0xfc4b2150 in strlen () from /usr/lib/libc.so.1
#1  0xfc51d704 in _ndoprnt () from /usr/lib/libc.so.1
#2  0xfc51fe24 in vsnprintf () from /usr/lib/libc.so.1
#3 0xfd19c07c in proto_tree_set_representation_value (pi=0xff850be8, format=0xfe6d2c48 "(%s) Type %u: Value (hex bytes): %s", ap=0xffbfdb50) at /Wireshark/source/epan/proto.c:3651
The basic problem is that Solaris' strlen() seg-faults if given a NULL pointer whereas a lot of other implementations just return 0.
Actually no, glibc strlen() also sigsegvs with strlen(0);

"Problem" is with Solaris vsnprintf implementation which doesn't check if
argument for %s is NULL. glibc implementation replaces NULL with "(null)"

Funny thing is that glib already have copy of gnulib formatting routines for system with non-C99 snprintf implementation [1] (like Windows), and glib team already fixed this issue for such systems. Solaris has good implementation so they won't fix it.
Oh, yeah, I remember finding that working implementation a long time ago 
You can read more at https://bugzilla.gnome.org/show_bug.cgi?id=167569
Funny, it has exactly my concern: works on Linux, crashes elsewhere.  Argh.

But it is encouraging as they indicate that Solaris is the only "significant" printf that is "good enough" except for this NULL pointer stuff. Well, except for those poor sods running "insignificant" printfs.
Is there a better way?  Or better yet, a proper solution?
- Replace g_snprintf() & friends with ws_snprintf() which accepts NULL %s

- I had the same issue with another project, and we created macro:
  #define __(x) ((x) ? (x) : "(null)")

and use it when passing possible-null-strings. It's still PITA but IMHO it looks a little better than doing it by hand.
I don't mind doing it by hand (and in some cases I prefer NOT having 
another layer of abstraction), but then we still need to have a way to 
(regularly) test for it.