Re: Segmentation fault found while fuzzing .pack file under 2.7.0.rc3

When we map in the .idx file, we do only minimum sanity checks to make sure that the .idx file itself has sorted fan-out. We do not check if the object names are sorted, so a bogus .idx could tell us that an object does not exist when it exists in the matching .pack, but that is harmless. Also an unsorted object names will not make our binary search run in circles while looking things up.

We do not check if the offset of individual objects are within the corresponding .pack file, either, and nth_packed_object_offset() does return the data read from .idx file that is not checked for sanity. use_pack(), which is the helper used by the callers of nth_packed_object_offset() that finds the offset in the packfile for each object, avoids allowing a read access to mapped pack data beyond the end of it, so it is OK to return bogus value that was read from the .idx file from this function, but there is one computation the function itself does using a possibly bogus value read from the disk: to find out where in the secondary offset table in the .idx file the offset in the packfile is stored.

---

 This is not even compile tested; I just wanted to prevent people
 from adding two unnecessary checks to this function following my
 analysis in the previous message.  I think returning bogus value
 stored in a crafted .idx file from this function is OK, as the
 offset will be first used by use_pack() and the sanity of the
 offset, relative to the packfile size, is checked there, and an
 offset that points to a random point in the packfile will be caught
 by the pack reading code, either by unpack_compressed_entry() or by
 patch_delta(), so that is also safe.

 We do need to check the unprotected access here.  Nobody else in
 the current codepath protects us from this access attempting to
 read an unmapped memory and segfault.

 sha1_file.c | 7 +++++++
 1 file changed, 7 insertions(+)