Re: [PATCH] pack-objects: re-validate data we copy from elsewhere.

From: Junio C Hamano <junkio@cox.net>
Date: Sep 3, 2006, 22:34 UTC
Message-ID: <7vodtwtw9v.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0609031456520.27779@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

Show 6 quoted lines

> On Sun, 3 Sep 2006, Junio C Hamano wrote:
>> 
>> Quite bad.  For the kernel archive of today (I usually am nearly
>> fully packed):
>
> Ok. Is it less painful if it just checks the zlib CRC...

I haven't checked myself but somebody said that zlib CRC is of preimage so we would need to incur inflate cost anyway if that is the case. But I think it may be a reasonable comproise to assume that an existing delta that inflates properly would apply to its base object, and if we can assume that we do not have to check the inflated xdelta data. Oops, not really, there is no check other than the pack overall SHA-1 checksum that protects the 20-byte base object name recorded in the pack.

> ... and that the SHA1 
> _exists_ for a delta - although I guess we check that indirectly by just 
> accepting the delta in the first place)? That combination should still be 
> a fairly strong check, of course.

Another thing the current check is _not_ doing is for this pathological case:

 - .idx table says the pack entry is N bytes

 - unpack_entry_gently() used in the revalidate code uses the
   usual codepath that says "here is the start of the pack
   entry; inflate using as much data as you need"; .idx is
   somehow wrong and it needed N+M bytes where 0 < M.

 - we copy out N bytes because we belive .idx.

-- 
VGER BF report: U 0.626721