Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 16, 2015, 03:19 UTC
Message-ID: <CAPig+cR9n=hT3F-0uDbJT_Z9REe83-kFKR5XB6pVrHRe0Z14eA@mail.gmail.com>
In-Reply-To: <20150915153637.GO29753@sigill.intra.peff.net>

On Tue, Sep 15, 2015 at 11:36 AM, Jeff King <peff@peff.net> wrote:

Show 25 quoted lines

> We sometimes sprintf into static buffers when we know that
> the size of the buffer is large enough to fit the input
> (either because it's a constant, or because it's numeric
> input that is bounded in size). Likewise with strcpy of
> constant strings.
>
> However, these sites make it hard to audit sprintf and
> strcpy calls for buffer overflows, as a reader has to
> cross-reference the size of the array with the input. Let's
> use xsnprintf instead, which communicates to a reader that
> we don't expect this to overflow (and catches the mistake in
> case we do).
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/builtin/merge-index.c b/builtin/merge-index.c
> index 1a1eafa..1d66111 100644
> --- a/builtin/merge-index.c
> +++ b/builtin/merge-index.c
> @@ -23,7 +23,7 @@ static int merge_entry(int pos, const char *path)
>                         break;
>                 found++;
>                 strcpy(hexbuf[stage], sha1_to_hex(ce->sha1));
> -               sprintf(ownbuf[stage], "%o", ce->ce_mode);
> +               xsnprintf(ownbuf[stage], sizeof(ownbuf[stage]), "%o", ce->ce_mode);

Interesting. I wonder if there are any (old/broken) compilers which would barf on this. If we care, perhaps sizeof(ownbuf[0]) instead?

>                 arguments[stage] = hexbuf[stage];
>                 arguments[stage + 4] = ownbuf[stage];
>         } while (++pos < active_nr);