git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: BUG: rev-parse segfault with invalid input

From
Elijah Newren <newren@gmail.com>
Date
May 23, 2018, 20:23 UTC
Message-ID
<CABPp-BFOwWvDpfLFa2yrUDU_3BU6F68oLTtO5FvQo8nr62_WtQ@mail.gmail.com>
In-Reply-To
<20180523195227.GT26695@zaya.teonanacatl.net>
On Wed, May 23, 2018 at 12:52 PM, Todd Zullinger <tmz@pobox.com> wrote:
Show 62 quoted lines
> Hi,
>
> Certain invalid input causes git rev-parse to crash rather
> than return a 'fatal: ambiguous argument ...' error.
>
> This was reported against the Fedora git package:
>
>     https://bugzilla.redhat.com/1581678
>
> Simple reproduction recipe and analysis, from the bug:
>
>     $ git init
>     Initialized empty Git repository in /tmp/t/.git/
>     $ git rev-parse ffffffffffffffffffffffffffffffffffffffff^@
>     Segmentation fault (core dumped)
>
>     gdb) break lookup_commit_reference
>     Breakpoint 1 at 0x555555609f00: lookup_commit_reference. (3 locations)
>     (gdb) r
>     Starting program: /usr/bin/git rev-parse ffffffffffffffffffffffffffffffffffffffff\^@
>     [Thread debugging using libthread_db enabled]
>     Using host libthread_db library "/lib64/libthread_db.so.1".
>
>     Breakpoint 1, lookup_commit_reference (oid=oid@entry=0x7fffffffd550) at commit.c:34
>     34              return lookup_commit_reference_gently(oid, 0);
>     (gdb) finish
>     Run till exit from #0  lookup_commit_reference (oid=oid@entry=0x7fffffffd550) at commit.c:34
>     try_parent_shorthands (arg=0x7fffffffdd44 'f' <repeats 40 times>) at builtin/rev-parse.c:314
>     314                     include_parents = 1;
>     Value returned is $1 = (struct commit *) 0x0
>     (gdb) c
>
>     (gdb) c
>     Continuing.
>
>     Program received signal SIGSEGV, Segmentation fault.
>     try_parent_shorthands (arg=0x7fffffffdd44 'f' <repeats 40 times>) at builtin/rev-parse.c:345
>     345             for (parents = commit->parents, parent_number = 1;
>     (gdb) l 336,+15
>     336             commit = lookup_commit_reference(&oid);
>     337             if (exclude_parent &&
>     338                 exclude_parent > commit_list_count(commit->parents)) {
>     339                     *dotdot = '^';
>     340                     return 0;
>     341             }
>     342
>     343             if (include_rev)
>     344                     show_rev(NORMAL, &oid, arg);
>     345             for (parents = commit->parents, parent_number = 1;
>     346                  parents;
>     347                  parents = parents->next, parent_number++) {
>     348                     char *name = NULL;
>     349
>     350                     if (exclude_parent && parent_number != exclude_parent)
>     351                             continue;
>
>     Looks like a null pointer check is missing.
>
> This occurs on master and as far back as 1.8.3.1 (what's in
> RHEL-6, I didn't try to test anything older).  Only a string
> with 40 valid hex characters and ^@, @-, of ^!  seems to
> trigger it.

Thanks for the detailed report. This apparently goes back to git-1.6.0 with commit 2122f8b963d4 ("rev-parse: Add support for the ^! and ^@ syntax", 2008-07-26). We aren't checking that the commit from lookup_commit_reference() is non-NULL before proceeding. Looks like it's simple to fix. I'll send a patch shortly...

Previous: Todd ZullingerNext: Todd Zullinger
Message 2 of 14 in “BUG: rev-parse segfault with invalid input”
  1. Todd ZullingerMay 23, 2018
  2. Elijah NewrenMay 23, 2018
  3. Todd ZullingerMay 23, 2018
  4. 1/2 t6101: add a test for rev-parse $garbage^@Elijah Newren, May 23, 2018
  5. 2/2 rev-parse: verify that commit looked up is not NULLElijah Newren, May 23, 2018
  6. Jeff KingMay 23, 2018
  7. rev-parse: check lookup'ed commit references for NULLElijah Newren, May 24, 2018
  8. Todd ZullingerMay 24, 2018
  9. Florian WeimerMay 24, 2018
  10. Jeff KingMay 24, 2018
  11. Junio C HamanoMay 25, 2018
  12. Todd ZullingerMay 23, 2018
  13. Todd ZullingerMay 23, 2018
  14. Jeff KingMay 23, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.