Re: BUG: rev-parse segfault with invalid input

[Todd Zullinger]

Hi,

Elijah Newren wrote:
> 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...

Thanks Elijah!  I thought it was likely to be a simple fix.
But I also don't know the area well and that kept me from
being too ambitious about suggesting a fix or the difficulty
of one. :)

-- 
Todd
~~
I believe in the noble, aristocratic art of doing absolutely nothing.
And someday, I hope to be in a position where I can do even less.

Re: BUG: rev-parse segfault with invalid input

[Elijah Newren]

On Wed, May 23, 2018 at 12:52 PM, Todd Zullinger  wrote:
> 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 ^@
> Segmentation fault (core dumped)
>
> gdb) break lookup_commit_reference
> Breakpoint 1 at 0x55609f00: lookup_commit_reference. (3 locations)
> (gdb) r
> Starting program: /usr/bin/git rev-parse 
> \^@
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
>
> Breakpoint 1, lookup_commit_reference (oid=oid@entry=0x7fffd550) 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=0x7fffd550) at commit.c:34
> try_parent_shorthands (arg=0x7fffdd44 'f' ) 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=0x7fffdd44 'f' ) at 
> builtin/rev-parse.c:345
> 345 for (parents = commit->parents, parent_number = 1;
> (gdb) l 336,+15
> 336 commit = lookup_commit_reference();
> 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, , 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...

BUG: rev-parse segfault with invalid input

[Todd Zullinger]

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 ^@
Segmentation fault (core dumped)

gdb) break lookup_commit_reference
Breakpoint 1 at 0x55609f00: lookup_commit_reference. (3 locations)
(gdb) r
Starting program: /usr/bin/git rev-parse 
\^@
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Breakpoint 1, lookup_commit_reference (oid=oid@entry=0x7fffd550) 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=0x7fffd550) at commit.c:34
try_parent_shorthands (arg=0x7fffdd44 'f' ) 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=0x7fffdd44 'f' ) at 
builtin/rev-parse.c:345
345 for (parents = commit->parents, parent_number = 1;
(gdb) l 336,+15
336 commit = lookup_commit_reference();
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, , 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.

-- 
Todd
~~
I don't mind arguing with myself. It's when I lose that it bothers me.
-- Richard Powers