git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Jeff King <peff@peff.net>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf
Date: Tue, 15 Sep 2015 23:19:21 -0400	[thread overview]
Message-ID: <CAPig+cR9n=hT3F-0uDbJT_Z9REe83-kFKR5XB6pVrHRe0Z14eA@mail.gmail.com> (raw)
In-Reply-To: <20150915153637.GO29753@sigill.intra.peff.net>

On Tue, Sep 15, 2015 at 11:36 AM, Jeff King <peff@peff.net> wrote:
> 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);

  parent reply	other threads:[~2015-09-16  3:19 UTC|newest]

Thread overview: 154+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-15 15:21 [PATCH 0/67] war on sprintf, strcpy, etc Jeff King
2015-09-15 15:23 ` [PATCH 01/67] show-branch: avoid segfault with --reflog of unborn branch Jeff King
2015-09-15 15:23 ` [PATCH 02/67] mailsplit: fix FILE* leak in split_maildir Jeff King
2015-09-15 15:23 ` [PATCH 03/67] archive-tar: fix minor indentation violation Jeff King
2015-09-15 15:24 ` [PATCH 04/67] fsck: don't fsck alternates for connectivity-only check Jeff King
2015-09-15 17:55   ` Johannes Schindelin
2015-09-16 18:04     ` Junio C Hamano
2015-09-16 18:12       ` Jeff King
2015-09-16 19:12         ` Junio C Hamano
2015-09-16 19:14           ` Eric Sunshine
2015-09-16 20:00             ` Jeff King
2015-09-15 15:24 ` [PATCH 05/67] add xsnprintf helper function Jeff King
2015-09-15 15:25 ` [PATCH 06/67] add git_path_buf " Jeff King
2015-09-15 15:25 ` [PATCH 07/67] strbuf: make strbuf_complete_line more generic Jeff King
2015-09-16  0:45   ` Eric Sunshine
2015-09-16  1:27     ` Junio C Hamano
2015-09-16  9:57       ` Jeff King
2015-09-16 15:11         ` Eric Sunshine
2015-09-15 15:26 ` [PATCH 08/67] add reentrant variants of sha1_to_hex and find_unique_abbrev Jeff King
2015-09-15 16:55   ` Ramsay Jones
2015-09-15 17:50     ` Jeff King
2015-09-16  1:32       ` Junio C Hamano
2015-09-16  8:15         ` Johannes Schindelin
2015-09-16 10:33           ` Jeff King
2015-09-16 17:06             ` Junio C Hamano
2015-09-16 17:23               ` Jeff King
2015-09-15 15:26 ` [PATCH 09/67] fsck: use strbuf to generate alternate directories Jeff King
2015-09-15 15:28 ` [PATCH 10/67] mailsplit: make PATH_MAX buffers dynamic Jeff King
2015-09-16  0:51   ` Eric Sunshine
2015-09-16 10:14     ` Jeff King
2015-09-16 10:25       ` Jeff King
2015-09-16 18:13         ` Junio C Hamano
2015-09-16 20:22           ` Jeff King
2015-09-15 15:28 ` [PATCH 11/67] trace: use strbuf for quote_crnl output Jeff King
2015-09-16  0:55   ` Eric Sunshine
2015-09-16 10:31     ` Jeff King
2015-09-16 15:16       ` Eric Sunshine
2015-09-15 15:29 ` [PATCH 12/67] progress: store throughput display in a strbuf Jeff King
2015-09-15 15:30 ` [PATCH 13/67] test-dump-cache-tree: avoid overflow of cache-tree name Jeff King
2015-09-15 15:31 ` [PATCH 14/67] compat/inet_ntop: fix off-by-one in inet_ntop4 Jeff King
2015-09-15 15:36 ` [PATCH 15/67] convert trivial sprintf / strcpy calls to xsnprintf Jeff King
2015-09-15 18:32   ` Ramsay Jones
2015-09-15 18:42     ` Jeff King
2015-09-15 19:15       ` Ramsay Jones
2015-09-15 20:38       ` Stefan Beller
2015-09-16  9:45         ` Jeff King
2015-09-16 18:20           ` Junio C Hamano
2015-09-16  1:34     ` Junio C Hamano
2015-09-16  3:19   ` Eric Sunshine [this message]
2015-09-16  9:48     ` Jeff King
2015-09-16 18:24       ` Junio C Hamano
2015-09-16 18:52         ` Jeff King
2015-09-16 19:07           ` Junio C Hamano
2015-09-16 19:19             ` Stefan Beller
2015-09-16 20:35               ` Jeff King
2015-09-16 20:32             ` Jeff King
2015-09-15 15:37 ` [PATCH 16/67] archive-tar: use xsnprintf for trivial formatting Jeff King
2015-09-15 15:38 ` [PATCH 17/67] use xsnprintf for generating git object headers Jeff King
2015-09-16 18:30   ` Junio C Hamano
2015-09-15 15:38 ` [PATCH 18/67] find_short_object_filename: convert sprintf to xsnprintf Jeff King
2015-09-15 15:39 ` [PATCH 19/67] stop_progress_msg: " Jeff King
2015-09-15 15:39 ` [PATCH 20/67] compat/hstrerror: convert sprintf to snprintf Jeff King
2015-09-15 15:39 ` [PATCH 21/67] grep: use xsnprintf to format failure message Jeff King
2015-09-15 15:40 ` [PATCH 22/67] entry.c: convert strcpy to xsnprintf Jeff King
2015-09-15 19:01   ` Ramsay Jones
2015-09-15 21:04     ` Stefan Beller
2015-09-15 15:41 ` [PATCH 23/67] add_packed_git: convert strcpy into xsnprintf Jeff King
2015-09-16 18:43   ` Junio C Hamano
2015-09-16 20:24     ` Jeff King
2015-09-15 15:42 ` [PATCH 24/67] http-push: replace strcat with xsnprintf Jeff King
2015-09-15 15:43 ` [PATCH 25/67] receive-pack: convert strncpy to xsnprintf Jeff King
2015-09-15 15:45 ` [PATCH 26/67] replace trivial malloc + sprintf /strcpy calls to xstrfmt Jeff King
2015-09-16  4:24   ` Eric Sunshine
2015-09-16 10:43     ` Jeff King
2015-09-15 15:45 ` [PATCH 27/67] config: use xstrfmt in normalize_value Jeff King
2015-09-15 15:46 ` [PATCH 28/67] fetch: replace static buffer with xstrfmt Jeff King
2015-09-15 15:47 ` [PATCH 29/67] use strip_suffix and xstrfmt to replace suffix Jeff King
2015-09-16  4:38   ` Eric Sunshine
2015-09-16 10:50     ` Jeff King
2015-09-16 15:20       ` Eric Sunshine
2015-09-15 15:48 ` [PATCH 30/67] ref-filter: drop sprintf and strcpy calls Jeff King
2015-09-16 19:33   ` Junio C Hamano
2015-09-15 15:48 ` [PATCH 31/67] help: drop prepend function in favor of xstrfmt Jeff King
2015-09-15 15:49 ` [PATCH 32/67] mailmap: replace strcpy with xstrdup Jeff King
2015-09-15 15:49 ` [PATCH 33/67] read_branches_file: " Jeff King
2015-09-16 19:52   ` Junio C Hamano
2015-09-16 20:42     ` Jeff King
2015-09-17 11:28       ` Jeff King
2015-09-17 11:32         ` Jeff King
2015-09-17 11:36         ` Jeff King
2015-09-17 15:38       ` Junio C Hamano
2015-09-17 16:24         ` Jeff King
2015-09-17 16:53           ` Junio C Hamano
2015-09-15 15:50 ` [PATCH 34/67] resolve_ref: use strbufs for internal buffers Jeff King
2015-09-15 15:51 ` [PATCH 35/67] upload-archive: convert sprintf to strbuf Jeff King
2015-09-15 15:52 ` [PATCH 36/67] remote-ext: simplify git pkt-line generation Jeff King
2015-09-16 20:18   ` Junio C Hamano
2015-09-16 21:23     ` Jeff King
2015-09-15 15:52 ` [PATCH 37/67] http-push: use strbuf instead of fwrite_buffer Jeff King
2015-09-15 15:53 ` [PATCH 38/67] http-walker: store url in a strbuf Jeff King
2015-09-15 15:54 ` [PATCH 39/67] sha1_get_pack_name: use " Jeff King
2015-09-15 15:56 ` [PATCH 40/67] init: use strbufs to store paths Jeff King
2015-09-15 15:57 ` [PATCH 41/67] apply: convert root string to strbuf Jeff King
2015-09-15 15:57 ` [PATCH 42/67] transport: use strbufs for status table "quickref" strings Jeff King
2015-09-15 15:58 ` [PATCH 43/67] merge-recursive: convert malloc / strcpy to strbuf Jeff King
2015-09-15 15:59 ` [PATCH 44/67] enter_repo: convert fixed-size buffers to strbufs Jeff King
2015-09-15 15:59 ` [PATCH 45/67] remove_leading_path: use a strbuf for internal storage Jeff King
2015-09-15 16:00 ` [PATCH 46/67] write_loose_object: convert to strbuf Jeff King
2015-09-16 21:27   ` Junio C Hamano
2015-09-16 21:39     ` Jeff King
2015-09-15 16:01 ` [PATCH 47/67] diagnose_invalid_index_path: use strbuf to avoid strcpy/strcat Jeff King
2015-09-15 16:02 ` [PATCH 48/67] fetch-pack: use argv_array for index-pack / unpack-objects Jeff King
2015-09-15 16:02 ` [PATCH 49/67] http-push: use an argv_array for setup_revisions Jeff King
2015-09-15 16:03 ` [PATCH 50/67] stat_tracking_info: convert to argv_array Jeff King
2015-09-15 16:04 ` [PATCH 51/67] daemon: use cld->env_array when re-spawning Jeff King
2015-09-15 16:05 ` [PATCH 52/67] use sha1_to_hex_to() instead of strcpy Jeff King
2015-09-16 21:51   ` Junio C Hamano
2015-09-16 21:54     ` Jeff King
2015-09-16 21:59       ` Junio C Hamano
2015-09-15 16:06 ` [PATCH 53/67] drop strcpy in favor of raw sha1_to_hex Jeff King
2015-09-18 19:24   ` Eric Sunshine
2015-09-18 19:29     ` Jeff King
2015-09-15 16:07 ` [PATCH 54/67] color: add overflow checks for parsing colors Jeff King
2015-09-18 18:54   ` Eric Sunshine
2015-09-18 19:01     ` Jeff King
2015-09-21 16:56       ` Junio C Hamano
2015-09-15 16:07 ` [PATCH 55/67] use alloc_ref rather than hand-allocating "struct ref" Jeff King
2015-09-15 16:09 ` [PATCH 56/67] avoid sprintf and strcpy with flex arrays Jeff King
2015-09-20 22:48   ` Eric Sunshine
2015-09-21 15:15     ` Jeff King
2015-09-21 17:11       ` Eric Sunshine
2015-09-21 17:19         ` Jeff King
2015-09-15 16:10 ` [PATCH 57/67] receive-pack: simplify keep_arg computation Jeff King
2015-09-18 18:43   ` Eric Sunshine
2015-09-18 18:49     ` Jeff King
2015-09-15 16:11 ` [PATCH 58/67] help: clean up kfmclient munging Jeff King
2015-09-15 16:11 ` [PATCH 59/67] prefer memcpy to strcpy Jeff King
2015-09-15 16:12 ` [PATCH 60/67] color: add color_set helper for copying raw colors Jeff King
2015-09-15 16:13 ` [PATCH 61/67] notes: document length of fanout path with a constant Jeff King
2015-09-15 16:13 ` [PATCH 62/67] convert strncpy to memcpy Jeff King
2015-09-15 16:14 ` [PATCH 63/67] fsck: drop inode-sorting code Jeff King
2015-09-15 16:14 ` [PATCH 64/67] Makefile: drop D_INO_IN_DIRENT build knob Jeff King
2015-09-15 16:15 ` [PATCH 65/67] fsck: use for_each_loose_file_in_objdir Jeff King
2015-09-15 16:16 ` [PATCH 66/67] use strbuf_complete to conditionally append slash Jeff King
2015-09-16 22:18   ` Junio C Hamano
2015-09-16 22:39     ` Jeff King
2015-09-16 22:54       ` Junio C Hamano
2015-09-16 22:57         ` Jeff King
2015-09-17 15:45           ` Junio C Hamano
2015-09-21  1:50   ` Eric Sunshine
2015-09-21 15:17     ` Jeff King
2015-09-15 16:16 ` [PATCH 67/67] name-rev: use strip_suffix to avoid magic numbers Jeff King
2015-09-16  1:54 ` [PATCH 0/67] war on sprintf, strcpy, etc Junio C Hamano
2015-09-16 10:35   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAPig+cR9n=hT3F-0uDbJT_Z9REe83-kFKR5XB6pVrHRe0Z14eA@mail.gmail.com' \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).