Am 15.03.2017 um 17:13 schrieb Jeff King: > On Wed, Mar 15, 2017 at 11:59:52AM -0400, Jeff King wrote: > >> I agree that detecting the situation in the meantime is a good idea. >> The patch above probably handles the bulk-checkin code path, I'd guess. >> It might be nice to have similar checks in other places, too: >> >> - when reading from an existing packfile >> >> Looks like we may already have such a check in >> unpack_object_header_buffer(). >> >> - when taking in new objects via index-pack or unpack-objects (to >> catch a fetch of a too-big object) >> >> I think index-pack.c:unpack_raw_entry() would want a similar check >> to what is in unpack_object_header_buffer(). > > Here are the results of a few quick experiments using two versions of > git, one built for 32-bit and one for 64-bit: > > $ git init > $ dd if=/dev/zero of=foo.zero bs=1M count=4097 > $ git32 add foo.zero > fatal: Cannot handle files this big > > That comes from the xsize_t() wrapper. I guess it wouldn't trigger on > Windows, though, because it is measuring size_t, not "unsigned long" (on > my 32-bit build they are the same, of course). > > $ git64 add foo.zero > $ git32 cat-file blob :foo.zero > error: bad object header > fatal: packed object df6f032f301d1ce40477eefa505f2fac1de5e243 (stored in > .git/objects/pack/pack-57d422f19904e9651bec43d10b7a9cd882de48ac.pack) is > corrupt > > So we notice, which is good. This is the message from > unpack_object_header_buffer(). It might be worth improving the error > message to mention the integer overflow. > > And here's what index-pack looks like: > > $ git32 index-pack --stdin <.git/objects/pack/*.pack > fatal: pack has bad object at offset 12: inflate returned -5 > > It's good that we notice, but the error message isn't great. What > happens is that we overflow the size integer, allocate a too-small > buffer, and then zlib complains when we run out of buffer but there's > still content to inflate. We probably ought to notice the integer > overflow in the first place and complain there.
Thanks for the pointers Peff. I'll try to come up with a patch in the next weeks. If somebody else steps in the meantime I'm not mad at all. Thomas
