On Wed, 14 Oct 2009, Shawn O. Pearce wrote: > Andy Isaacson <adi@xxxxxxxxxxxxx> wrote: > > We're looping in unpack_compressed_entry, adding a fprintf immediately > > after the call to git_inflate() shows: > > Thanks, that was really quite helpful. Junio/Nico, I think we can > just apply this patch to maint and include it in the next release: > > --8<-- > [PATCH] sha1_file: Fix infinite loop when pack is corrupted > > Some types of corruption to a pack may confuse the deflate stream > which stores an object. In Andy's reported case a 36 byte region > of the pack was overwritten, leading to what appeared to be a valid > deflate stream that was trying to produce a result larger than our > allocated output buffer could accept. > > Z_BUF_ERROR is returned from inflate() if either the input buffer > needs more input bytes, or the output buffer has run out of space. > Previously we only considered the former case, as it meant we needed > to move the stream's input buffer to the next window in the pack. > > We now abort the loop if inflate() returns Z_BUF_ERROR without > consuming the entire input buffer it was given, or has filled > the entire output buffer but has not yet returned Z_STREAM_END. > Either state is a clear indicator that this loop is not working > as expected, and should not continue. > > This problem cannot occur with loose objects as we open the entire > loose object as a single buffer and treat Z_BUF_ERROR as an error. > > Reported-by: Andy Isaacson <adi@xxxxxxxxxxxxx> > Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx> Acked-by: Nicolas Pitre <nico@xxxxxxxxxxx> This is unfortunate that making a test case for this isn't exactly trivial. > --- > sha1_file.c | 4 ++++ > 1 files changed, 4 insertions(+), 0 deletions(-) > > diff --git a/sha1_file.c b/sha1_file.c > index 4ea0b18..4cc8939 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -1357,6 +1357,8 @@ unsigned long get_size_from_delta(struct packed_git *p, > in = use_pack(p, w_curs, curpos, &stream.avail_in); > stream.next_in = in; > st = git_inflate(&stream, Z_FINISH); > + if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out)) > + break; > curpos += stream.next_in - in; > } while ((st == Z_OK || st == Z_BUF_ERROR) && > stream.total_out < sizeof(delta_head)); > @@ -1594,6 +1596,8 @@ static void *unpack_compressed_entry(struct packed_git *p, > in = use_pack(p, w_curs, curpos, &stream.avail_in); > stream.next_in = in; > st = git_inflate(&stream, Z_FINISH); > + if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out)) > + break; > curpos += stream.next_in - in; > } while (st == Z_OK || st == Z_BUF_ERROR); > git_inflate_end(&stream); > -- > 1.6.5.52.g0ff2e > > -- > Shawn. > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html