Re: [RFC PATCH] unpack-trees: watch for out-of-range index position

From: Emily Shaffer <emilyshaffer@google.com>
To: Jeff King <peff@peff.net>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [RFC PATCH] unpack-trees: watch for out-of-range index position
Date: Fri, 10 Jan 2020 15:07:06 -0800	[thread overview]
Message-ID: <20200110230706.GH181522@google.com> (raw)
In-Reply-To: <20200110063741.GA409153@coredump.intra.peff.net>

On Fri, Jan 10, 2020 at 01:37:41AM -0500, Jeff King wrote:
> On Thu, Jan 09, 2020 at 02:46:41PM -0800, Emily Shaffer wrote:
> 
> > > Perhaps. The integrity check only protects against an index that was
> > > modified after the fact, not one that was generated by a buggy Git. I'm
> > > not sure we know how the index that led to this patch got into this
> > > state (though it sounds like Emily has a copy and could check the hash
> > > on it), but other cache-tree segfault I found recently was with an index
> > > with an intact integrity hash.
> > 
> > Yeah, I can do that, although I'm not sure how. The index itself is very
> > small - it only contains one file and one tree extension - so I'll go
> > ahead and paste some poking and prodding, and if it's not what you
> > wanted then please let me know what else to run.
> 
> I was thinking you would run something like:
> 
>   size=$(stat --format=%s "$file")
>   actual=$(head -c $(($size-20)) "$file" | sha1sum | awk '{print $1}')
>   expect=$(xxd -s -20 -g 20 -c 20 "$file" | awk '{print $2}')
>   if test "$actual" = "$expect"; then
>           echo "OK ($actual)"
>   else
>           echo "FAIL ($actual != $expect)"
>   fi
> 
> to manually check the sha1.

Unsurprising given your mail, yeah, this looks OK when I run it against
the repo in question.

> So this bogus index was probably actually created by Git, not an
> after-the-fact byte corruption.

Disappointingly, the repro repo we got was aggressively redacted - I
don't have any reflogs to look through and try and get a hint of what
happened, and I imagine the reporter has moved on with their life enough
that we can't get something useful from there now.

 - Emily