Re: [RESEND PATCH 1/2] Fix builtin-fetch memory corruption by not overstepping array

["Shawn O. Pearce" <spearce@spearce.org>]

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Unfortunately we cannot continue to NULL terminate the array at
> this point as the array may only have been allocated large enough
> to match the input of nr_heads.  If there are no duplicates than
> we would be trying to store NULL into heads[nr_heads] and that may
> be outside of the array.
...
> diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
> index b0936cc..2977a94 100644
> --- a/builtin-fetch-pack.c
> +++ b/builtin-fetch-pack.c
> @@ -654,7 +654,6 @@ static int remove_duplicates(int nr_heads, char **heads)
>  			heads[dst] = heads[src];
>  		dst++;
>  	}
> -	heads[dst] = 0;
>  	return dst;
>  }
>  

This fortunately does not appear to be a bug in `maint`, `master`,
or `next`.  It appears to be isolated to the builtin-fetch topic.
The builtin-fetch topic changed fetch-pack to be builtin-fetch-pack
and in doing so changed things such that the heads array might not
have room for this final entry.

In maint/master/next the heads array is actually backed by the
argv passed into main by the kernel, and that does have an existing
NULL terminator, which the above "heads[dst] = 0" is simply going
to overwrite if "heads == nr_heads".

-- 
Shawn.