Re: SHA1 collision in production repo?! (probably not)
- From
Jeff King <peff@peff.net>
- Date
- Apr 1, 2017, 08:03 UTC
- Message-ID
- <20170401080349.lccextuc3l6fgs6j@sigill.intra.peff.net>
- In-Reply-To
- <xmqq37dtxi36.fsf@gitster.mtv.corp.google.com>
On Fri, Mar 31, 2017 at 02:16:29PM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> Before we forget... > > -- >8 -- > From: Jeff King <peff@peff.net> > > When sha1_loose_object_info() finds that a loose object file cannot > be stat(2)ed or mmap(2)ed, it returns -1 to signal an error to the > caller. However, if it found that the loose object file is corrupt > and the object data cannot be used from it, it forgets to return -1. > > This can confuse the caller, who may be lead to mistakenly think > that there is a loose object and possibly gets an incorrect type and > size from the function. The SHA-1 collision detection codepath, for > example, when it gets an object over the wire and tries to see the > data is the same as what is available as a loose object locally, can > get confused when the loose object is correupted due to this bug.
Unfortunately this isn't quite right. I was able to reproduce the problem that Lars saw, and this patch doesn't fix it.
So here's a two-patch series. The first fixes the problem described above, along with a simpler test that demonstrates it. The second fixes Lars's problem on top.
I know you're heading out for a week, so I'll post it now for review, and then hold and repost when you get back.
[1/2]: sha1_loose_object_info: return error for corrupted objects [2/2]: index-pack: detect local corruption in collision check
builtin/index-pack.c | 2 ++ sha1_file.c | 2 +- t/t1060-object-corruption.sh | 24 ++++++++++++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-)
-Peff