]> git.ipfire.org Git - thirdparty/git.git/commit
strbuf_getwholeline: NUL-terminate getdelim buffer on error
authorJeff King <peff@peff.net>
Sat, 5 Mar 2016 18:43:30 +0000 (13:43 -0500)
committerJunio C Hamano <gitster@pobox.com>
Sat, 5 Mar 2016 18:57:37 +0000 (10:57 -0800)
commitb70904306fc30857fa3638d2bce7ae0ad1251e23
tree5ffc4cd6567e5d35a00c94508f8b79102ccc578e
parent326e5bc91eecf73234ead29636207bc516573e79
strbuf_getwholeline: NUL-terminate getdelim buffer on error

Commit 0cc30e0 (strbuf_getwholeline: use getdelim if it is
available, 2015-04-16) tries to clean up after getdelim()
returns EOF, but gets one case wrong, which can lead in some
obscure cases to us reading uninitialized memory.

After getdelim() returns -1, we re-initialize the strbuf
only if sb->buf is NULL. The thinking was that either:

  1. We fed an existing allocated buffer to getdelim(), and
     at most it would have realloc'd, leaving our NUL in
     place.

  2. We didn't have a buffer to feed, so we gave getdelim()
     NULL; sb->buf will remain NULL, and we just want to
     restore the empty slopbuf.

But that second case isn't quite right. getdelim() may
allocate a buffer, write nothing into it, and then return
EOF. The resulting strbuf rightfully has sb->len set to "0",
but is missing the NUL terminator in the first byte.

Most call-sites are fine with this. They see the EOF and
don't bother looking at the strbuf. Or they notice that
sb->len is empty, and don't look at the contents. But
there's at least one case that does neither, and relies on
parsing the resulting (possibly zero-length) string:
fast-import. You can see this in action with the new test
(though we probably only notice failure there when run with
--valgrind or ASAN).

We can fix this by unconditionally resetting the strbuf when
we have a buffer after getdelim(). That fixes case 2 above.
Case 1 is probably already fine in practice, but it does not
hurt for us to re-assert our invariants (especially because
we are relying on whatever getdelim() happens to do, which
may vary from platform to platform). Our fix covers that
case, too.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
strbuf.c
t/t9300-fast-import.sh