Re: [PATCH] Avoid infinite loop in malformed packfiles

[Junio C Hamano <gitster@pobox.com>]

Ori Bernstein <ori@eigenstate.org> writes:

> diff --git a/packfile.c b/packfile.c
> index 6ab5233613..321e002c50 100644
> --- a/packfile.c
> +++ b/packfile.c
> @@ -1715,6 +1716,12 @@ void *unpack_entry(struct repository *r, struct packed_git *p, off_t obj_offset,
>  			break;
>  		}
>  
> +		if (delta_stack_nr > UNPACK_ENTRY_STACK_LIMIT) {
> +			error("overlong delta chain at offset %jd from %s",
> +			      (uintmax_t)curpos, p->pack_name);

The "j" length field is not used anywhere in the codebase for
portability concerns, I think.  "d" is for signed, but curpos
is an unsigned off_t.  I think

	"... %"PRIuMAX" from %s", (uintmax_t)curpos, ...

would match how we write this kind of thing everywhere else in the
code, e.g. showing obj_offset in packed_to_object_type() in the same
file in an error message.

> @@ -1633,6 +1633,7 @@ static void write_pack_access_log(struct packed_git *p, off_t obj_offset)
>  
>  int do_check_packed_object_crc;
>  
> +#define UNPACK_ENTRY_STACK_LIMIT 10000
>  #define UNPACK_ENTRY_STACK_PREALLOC 64
>  struct unpack_entry_stack_ent {
>  	off_t obj_offset;

What escape hatch would the end-users have when they have a
legitimate packfile that has a truly deep delta chain, by the way?

Thanks.