Re: [PATCH] fsckObjects tests: show how v2.17.1 can exploit downstream

From: Jeff King <peff@peff.net>
Date: Jun 1, 2018, 05:57 UTC
Message-ID: <20180601055715.GD8292@sigill.intra.peff.net>
In-Reply-To: <xmqq4linb7vb.fsf@gitster-ct.c.googlers.com>

On Fri, Jun 01, 2018 at 10:42:00AM +0900, Junio C Hamano wrote:

Show 18 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > I haven't tested it, but I suspect that doing multiple fetches could
> > result in passing bad objects through a fetch.fsckObjects filter.
> > Because the objects aren't quarantined on fetch, and because
> > fsck_finish() requires the objects to be installed into place, they may
> > ...
> > I think in the long run fetch should implement a similar quarantine
> > procedure to what happens on push.
> 
> Interesting.
> 
> I wonder if we can teach quickfetch codepath to notice the presence
> of fsckObjects, instead of doing a full quarantine.  We can easily
> enumerate those objects that were already in the object database but
> outside of the reachability chain before we pretend that we fetched
> them and make them reachable, and check the content integrity of
> them, no?

Yes, we could. But it kind of feels like plugging holes in the dike. That saves "fetch" from referencing them accidentally, but other git programs may see and react to them. E.g., you're just an "update-ref" away from referencing the bad history. I don't expect that most attackers can then convince a victim to reference the rejected objects, but it feels a lot more hand-wavy than just saying "we don't let these objects into the repository in the first place".

-Peff