git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Subject: [PATCH v2 0/68] war on sprintf
Date: Thu, 24 Sep 2015 17:02:25 -0400	[thread overview]
Message-ID: <20150924210225.GA23624@sigill.intra.peff.net> (raw)

This is a revised version of the series I sent earlier[1].

For those just joining us, the goal is to remove sprintf, strcpy, etc,
to make it easier to audit the code base for buffer overflows. I've been
addressing review comments for individual patches as the discussion
progressed, so there's nothing here that hasn't been on the list (and I
think I've addressed all of the comments from the first round of
review).

Here's a list of the commits, with an overview[2] of the changes from v1
(patches without comment are the same as in v1):

  [01/68]: show-branch: avoid segfault with --reflog of unborn branch
  [02/68]: mailsplit: fix FILE* leak in split_maildir
  [03/68]: archive-tar: fix minor indentation violation
  [04/68]: fsck: don't fsck alternates for connectivity-only check
  [05/68]: add xsnprintf helper function
  [06/68]: add git_path_buf helper function
  [07/68]: strbuf: make strbuf_complete_line more generic

    Docstring now makes clear the behavior when the strbuf is empty.

  [08/68]: add reentrant variants of sha1_to_hex and find_unique_abbrev

    These are now called sha1_to_hex_r, etc. The re-entrancy conditions
    are documented.

  [09/68]: fsck: use strbuf to generate alternate directories
  [10/68]: mailsplit: make PATH_MAX buffers dynamic

    Use xstrfmt consistently instead of sometimes using a strbuf.

  [11/68]: trace: use strbuf for quote_crnl output

    Further cleanup of "p2" variable in quote_crnl.

  [12/68]: progress: store throughput display in a strbuf
  [13/68]: test-dump-cache-tree: avoid overflow of cache-tree name
  [14/68]: compat/inet_ntop: fix off-by-one in inet_ntop4
  [15/68]: convert trivial sprintf / strcpy calls to xsnprintf

    Prefer "fixed-size" to "static" for clarity in commit message.

  [16/68]: archive-tar: use xsnprintf for trivial formatting
  [17/68]: use xsnprintf for generating git object headers
  [18/68]: find_short_object_filename: convert sprintf to xsnprintf
  [19/68]: stop_progress_msg: convert sprintf to xsnprintf

    Ditto on "fixed-size" versus "static".

  [20/68]: compat/hstrerror: convert sprintf to snprintf
  [21/68]: grep: use xsnprintf to format failure message
  [22/68]: entry.c: convert strcpy to xsnprintf
  [23/68]: add_packed_git: convert strcpy into xsnprintf

    Drop useless comment. Add comment on magic strlen().

  [24/68]: http-push: replace strcat with xsnprintf
  [25/68]: receive-pack: convert strncpy to xsnprintf
  [26/68]: replace trivial malloc + sprintf / strcpy calls with xstrfmt

    Include NUL in base64 of imap-send cram response. My guess is this
    is a bug in what we send that is overlooked by most servers, but I'd
    prefer to be conservative here and keep the behavior the same.

  [27/68]: config: use xstrfmt in normalize_value
  [28/68]: fetch: replace static buffer with xstrfmt
  [29/68]: use strip_suffix and xstrfmt to replace suffix
  [30/68]: ref-filter: drop sprintf and strcpy calls
  [31/68]: help: drop prepend function in favor of xstrfmt
  [32/68]: mailmap: replace strcpy with xstrdup
  [33/68]: read_branches_file: simplify string handling

    This goes much further than the original in cleaning up the use of a
    static buffer. From what I sent earlier during review, I dropped
    strbuf_read_file in favor of strbuf_getline, to keep the behavior
    identical to the original.

  [34/68]: read_remotes_file: simplify string handling

    New in this iteration; cleanups to match those in 33/68.

  [35/68]: resolve_ref: use strbufs for internal buffers
  [36/68]: upload-archive: convert sprintf to strbuf
  [37/68]: remote-ext: simplify git pkt-line generation

    The v1 of this patch was totally buggy. This is a rewrite to use
    packet_write(), which has several advantages, and includes tests.

  [38/68]: http-push: use strbuf instead of fwrite_buffer
  [39/68]: http-walker: store url in a strbuf
  [40/68]: sha1_get_pack_name: use a strbuf
  [41/68]: init: use strbufs to store paths
  [42/68]: apply: convert root string to strbuf
  [43/68]: transport: use strbufs for status table "quickref" strings
  [44/68]: merge-recursive: convert malloc / strcpy to strbuf
  [45/68]: enter_repo: convert fixed-size buffers to strbufs
  [46/68]: remove_leading_path: use a strbuf for internal storage
  [47/68]: write_loose_object: convert to strbuf

    Clarify comment on subtle mkstemp behavior.

  [48/68]: diagnose_invalid_index_path: use strbuf to avoid strcpy/strcat
  [49/68]: fetch-pack: use argv_array for index-pack / unpack-objects
  [50/68]: http-push: use an argv_array for setup_revisions
  [51/68]: stat_tracking_info: convert to argv_array
  [52/68]: daemon: use cld->env_array when re-spawning
  [53/68]: use sha1_to_hex_r() instead of strcpy

    Use "_r" versions to match change in patch 8.

  [54/68]: drop strcpy in favor of raw sha1_to_hex

    More history in the commit message, courtesy of Eric.

  [55/68]: color: add overflow checks for parsing colors

    Be more careful with OUT() macro.

  [56/68]: use alloc_ref rather than hand-allocating "struct ref"
  [57/68]: avoid sprintf and strcpy with flex arrays

    Trivial typo-fix in comment.

  [58/68]: receive-pack: simplify keep_arg computation

    Another s/static/fixed-size/.

  [59/68]: help: clean up kfmclient munging
  [60/68]: prefer memcpy to strcpy
  [61/68]: color: add color_set helper for copying raw colors
  [62/68]: notes: document length of fanout path with a constant
  [63/68]: convert strncpy to memcpy
  [64/68]: fsck: drop inode-sorting code
  [65/68]: Makefile: drop D_INO_IN_DIRENT build knob
  [66/68]: fsck: use for_each_loose_file_in_objdir
  [67/68]: use strbuf_complete to conditionally append slash

    Drop bogus conversion from imap-send.

  [68/68]: name-rev: use strip_suffix to avoid magic numbers

Hopefully that makes reviewing v2 a little less painful for those who
already slogged through v1. Thanks for your patience. :)

-Peff

[1] Gmane seems down, but once it's back up you should be able to get to
    v1 at: http://mid.gmane.org/20150915152125.GA27504@sigill.intra.peff.net

[2] Keeping track of the tweaks to this many patches would have been
    near impossible without Thomas Rast's git-tbdiff tool:

      https://github.com/trast/tbdiff

    I highly recommend it for people handling multiple versions of a
    long series.

             reply	other threads:[~2015-09-24 21:02 UTC|newest]

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20150924210225.GA23624@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).