[MPlayer-cvslog] r25824 - trunk/stream/stream_cddb.c

[Rich Felker]

On Sun, Jan 20, 2008 at 04:24:16PM -0500, Rich Felker wrote:
> On Sun, Jan 20, 2008 at 09:58:02PM +0100, rtogni wrote:
> > Author: rtogni
> > Date: Sun Jan 20 21:58:02 2008
> > New Revision: 25824
> > 
> > Log:
> > Prevent possible buffer overflow on album_title[]
> > Bassed on a patch by Adam Bozanich abozanich musecurity com
> > 
> > 
> > Modified:
> >    trunk/stream/stream_cddb.c
> > 
> > Modified: trunk/stream/stream_cddb.c
> > ==============================================================================
> > --- trunk/stream/stream_cddb.c	(original)
> > +++ trunk/stream/stream_cddb.c	Sun Jan 20 21:58:02 2008
> > @@ -58,6 +58,7 @@
> >  #include "version.h"
> >  #include "stream.h"
> >  #include "network.h"
> > +#include "libavutil/intreadwrite.h"
> >  
> >  #define DEFAULT_FREEDB_SERVER	"freedb.freedb.org"
> >  #define DEFAULT_CACHE_DIR	"/.cddb/"
> > @@ -503,8 +504,9 @@ cddb_parse_matches_list(HTTP_header_t *h
> >  		} else {
> >  			len = ptr2-ptr+1;
> >  		}
> > +		len = FFMIN(sizeof(album_title) - 1, len);
> >  		strncpy(album_title, ptr, len);
> > -		album_title[len-2]='\0';
> > +		album_title[len]='\0';
> 
> This is just a nasty workaround for misuse of strncpy. Use strlcpy
> (av_strlcpy?) or even snprintf instead...

Try this:

snprintf(album_title, sizeof(album_title), "%.*s", len, ptr);

This conveys the intended semantics in one concise line: generate a
string from the first len (or fewer) bytes at ptr and store it in
album_title, which has a max size of sizeof(album_title) bytes.

Rich