[vlc-devel] [PATCH] playlist service discovery NULL pointer dereference

[Alexander Gall]

Pierre,

On Mon, 20 Aug 2007 19:09:30 +0200, Pierre d'Herbemont <pdherbemont at free.fr> said:

> You are right, psz_cat can be NULL (and in fact is most of the time),  
> however a good implementation of xxprintf usually check for NULL  
> string pointers, and print "(null)" accordingly. That's why I wonder  
> on what system are you?

Solaris.  The man page of snprintf() says for "%s":

  The argument must be a pointer to an array of  char...
  ... An  argument  with a null value will yield undefined results.

I guess it's not safe to assume that every implementation checks the
arguments.  I'm beginning to understand why these bugs only get
noticed by me, though :-)

> I'll apply more or less the same patch, but we may want to catch the  
> problem at a lower level, and provide our own safe implementation of  
> vaprintf.

That would probably be the easiest way.

> Could you be more specific about the other NULL pointers bad  
> handling? 

Two quick examples: in src/libvlc-common.c:Usage(), around line 1619

   1619             /* We wrap the rest of the output */
   1620             sprintf( psz_buffer, "%s%s", p_item->psz_text, psz_suf );

p_item->psz_text is not guaranteed to be non-NULL.  There is at least
one module that does not define that help text (I forgot which one it
is).  Defensive programming would dictate that one cannot rely on
every author of a module to define all necessary objects.  My VLC
crashes when called wit the --longhelp option.  I haven't reported
this one yet :-)

There is another such bug in the code that I sent a (unrelated) patch
for on August 10 (subject "[PATCH] find muxer for stream_out
standard").

The SDP parser used to be very bad at dealing with unexpected input
and crashed a lot with NULL pointer dereferences (not enough testing
with real life data, I guess).  It appears to have been rewritten in
0.9.0 and seems much more robust now.

> Usually such crash are not hard to resolve, unless they  
> hide a much bigger trouble.

Yes, but a "clean" crash with an assertion failure is still a bit
better than getting a plain segmentation violation.

-- 
Alex

_______________________________________________
vlc-devel mailing list
To unsubscribe or modify your subscription options:
http://mailman.videolan.org/listinfo/vlc-devel