Re: SHA1 collision in production repo?! (probably not)

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 31, 2017, 21:16 UTC
Message-ID: <xmqq37dtxi36.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170331174515.j2ruifuigskyvucc@sigill.intra.peff.net>

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.

---
 sha1_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1_file.c b/sha1_file.c
index 71063890ff..368c89b5c3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2952,7 +2952,7 @@ static int sha1_loose_object_info(const unsigned char *sha1,
 	if (status && oi->typep)
 		*oi->typep = status;
 	strbuf_release(&hdrbuf);
-	return 0;
+	return (status < 0) ? status : 0;
 }
 
 int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, unsigned flags)