Re: New NULL dereference in sequence.c

From: Neil Horman <nhorman@tuxdriver.com>
Date: May 2, 2012, 17:34 UTC
Message-ID: <20120502173436.GA26732@hmsreliant.think-freely.org>
In-Reply-To: <4FA12E42.201@lsrfire.ath.cx>

On Wed, May 02, 2012 at 02:53:22PM +0200, René Scharfe wrote:

Show 33 quoted lines

> [cc:ing the author of this commit]
> 
> Am 02.05.2012 10:45, schrieb Michael Mueller:
> >Hi all,
> >
> >The last defect Sentry picked up has been removed, yay!  At the same
> >time, a new NULL dereference bug appeared, in sequencer.c:
> >
> >     static int is_index_unchanged(void)
> >     {
> >         unsigned char head_sha1[20];
> >         struct commit *head_commit;
> >
> >         if (!resolve_ref_unsafe("HEAD", head_sha1, 1, NULL))
> >             return error(_("Could not resolve HEAD commit\n"));
> >
> >         head_commit = lookup_commit(head_sha1);
> >         if (!head_commit || parse_commit(head_commit))
> >             return error(_("could not parse commit %s\n"),
> >                      sha1_to_hex(head_commit->object.sha1));
> >
> >In the last line quoted above, head_commit may be NULL, and it is
> >dereferenced in the call to sha1_to_hex.  Assuming lookup_commit(head_sha1)
> >can return NULL, this function will segfault.
> >
> >Introduced here:
> >https://github.com/gitster/git/commit/b27cfb0#sequencer.c
> 
> Similar code in builtin/commit.c just reports "could not parse HEAD
> commit" without any hash and thus no pointer dereference.
> 
> René
>

Have you actually seen this problem occur? It seems to me that the only way head_commit could be NULL is in the event that HEAD wasn't a commit object, whcih I don't think can be the case unless something else is very wrong with your tree.

Neil