From: Jeff King <peff@peff.net>
Date: Tue, 15 Sep 2015 18:42:11 GMT
Subject: Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf
Message-ID: <20150915184211.GA31939@sigill.intra.peff.net>
In-Reply-To: <55F8643D.6040800@ramsayjones.plus.com>

On Tue, Sep 15, 2015 at 07:32:29PM +0100, Ramsay Jones wrote:

> > diff --git a/archive-tar.c b/archive-tar.c
> > index b6b30bb..d543f93 100644
> > --- a/archive-tar.c
> > +++ b/archive-tar.c
> > @@ -301,7 +301,7 @@ static int write_global_extended_header(struct archiver_args *args)
> >  	memset(&header, 0, sizeof(header));
> >  	*header.typeflag = TYPEFLAG_GLOBAL_HEADER;
> >  	mode = 0100666;
> > -	strcpy(header.name, "pax_global_header");
> > +	xsnprintf(header.name, sizeof(header.name), "pax_global_header");
> 
> How about using strlcpy() instead? Thus:
> 
> -	strcpy(header.name, "pax_global_header");
> +	strlcpy(header.name, "pax_global_header", sizeof(header.name));
> 
> Ditto for other similar (strcpy->xsnprintf) hunks below.

That misses the "assert" behavior of xsnprintf. We are preventing
overflow here, but also truncation. What should happen if
"pax_global_header" does not fit in header.name? I think complaining
loudly and immediately is the most helpful thing, because it is surely a
programming error.

We could make xstrlcpy(), of course, but I don't see much point when
xsnprintf does the same thing (and more).

-Peff