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. -Peff