Re: New index-pack "keep" violates "never overwrite"

[Nicolas Pitre <nico@cam.org>]

On Tue, 20 Mar 2007, Shawn O. Pearce wrote:

> This is something that has been bothering me for several weeks now.
> 
> Waaaaaaay back Git was considered to be secure as it never overwrote
> an object it already had.  This was ensured by always unpacking the
> packfile received over the network (both in fetch and receive-pack)
> and our already existing logic to not create a loose object for an
> object we already have.
> 
> Lately however we keep "large-ish" packfiles on both fetch and push
> by running them through index-pack instead of unpack-objects.  This
> would let an attacker perform a birthday attack. 

This has been nagging me as well since I made the index-pack changes.  
But I dismissed that possibility as pure paranoia.

[...]
> The fix does appear to be simple.  Just don't write the existing
> object to the output packfile.  But really that is a lot more like
> what unpack-objects does: buffer deltas we cannot resolve yet, and
> only write out what we cannot find through our ODB.  The logic in
> index-pack ain't built for that...

Nah.  You would have to fix every delta base offsets up etc. and make 
the whole thing into a complex mess.

Instead, we only have to check for the local presence of every objects 
in the pack, and when one packed object does exist locally and 
doesn't match the pack content we simply need to bail out with great 
distress.  If we really get a SHA1 collision, evil or not, we simply 
just cannot continue as if nothing happened.

Patch to follow...


Nicolas