Subject: Re: [PATCH] [GSOC] ref-filter: solve bugs caused by enumeration

From: Junio C Hamano <[email-masked]>
Date: Fri, 07 May 2021 06:20:09 +0900

ZheNing Hu <[email-masked]> writes:

> ... now I think something like this will be more secure. In the
> future, other people may be grateful for such a choice. :)
>
> @@ -1730,7 +1759,7 @@ static int populate_value(struct ref_array_item
> *ref, struct strbuf *err)
>                         else
>                                 v->s = xstrdup("");
>                         continue;
> -               } else if (starts_with(name, "push")) {
> +               } else if (starts_with(name, "push") &&
> atom->u.remote_ref.push) {

Hmph, I do no think that would be futureproof at all.  If a new atom
"pushe" gets added, it is not all that unlikely that it would add
its own member to the same union.  name here would be "pushe" and
starts with "push", and upon parsing "pushe", its own member in the
union, atom->u.X, would have been assigned to, but the code we see
here still accesses atom->u.remote_ref.*, so you still have the same
problem you started to solve, no?

The check we use in remote_ref_atom_parser() to see if the atom is
about pushing, i.e.

	if (!strcmp(atom->name, "push") || starts_with(atom->name, "push:"))

is unlikely to be invalidated in future changes, as this is very
much end-user facing and changing the condition will break existing
scripts, so that is what I was expecting you to use instead.