Re: [PATCH v2 1/2] fsck: use strbuf_getline() to read skiplist file

From: Jeff King <peff@peff.net>
Date: Aug 27, 2018, 23:00 UTC
Message-ID: <20180827230040.GA10402@sigill.intra.peff.net>
In-Reply-To: <aaf03ba2-1369-2dc3-c3ec-739a1c56606e@web.de>

On Sat, Aug 25, 2018 at 08:50:28PM +0200, René Scharfe wrote:

> buffer is unlikely to contain a NUL character, so printing its contents
> using %s in a die() format is unsafe (detected with ASan).

Having mostly forgotten about our earlier discussion, I got confused by this, thinking the problem was that there is some issue with missing NULs in the input.

But it is really just:

  We read() into a buffer and on error format the contents using "%s".
  But read() does not NUL-terminate, so die() might walk past the end of
  the buffer.

We _might_ be saved by a NUL in the input, but that is not the primary concern. ;)

Not worth a re-roll on its own, but since there is some other discussion, I thought I'd mention my confusion. :)

Show 5 quoted lines

> Added error check.
> Hopefully fixed my MUA config..
> 
>  fsck.c | 25 ++++++++++++-------------
>  1 file changed, 12 insertions(+), 13 deletions(-)

Patch itself looks good to me.

-Peff