Re: [PATCH v3] object-name: avoid use-after-free in get_oid_with_context_1()

Shlok Kulshreshtha <[email protected]> writes:

> When a ":<path>" argument names a relative path, resolve_relative_path()
> returns a newly allocated string and "cp" is pointed at it:
>
> 	new_path = resolve_relative_path(repo, cp);
> 	if (!new_path) {
> 		namelen = namelen - (cp - name);
> 	} else {
> 		cp = new_path;
> 		namelen = strlen(cp);
> 	}
>
> From there on "cp" and "new_path" name the same allocation. Later the
> memory location that "new_path" points to is freed.
>
> 	free(new_path);
> 	if (reject_tree_in_index(repo, only_to_die, ce, stage, prefix, cp))

Nicely described and ...

> diff --git a/object-name.c b/object-name.c
> index 83efba0ba6..026ff8c6dd 100644
> --- a/object-name.c
> +++ b/object-name.c
> @@ -1803,13 +1803,15 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
>  			    memcmp(ce->name, cp, namelen))
>  				break;
>  			if (ce_stage(ce) == stage) {
> +				int ret = reject_tree_in_index(repo, only_to_die, ce,
> +							       stage, prefix, cp);
> +
> +				if (!ret) {
> +					oidcpy(oid, &ce->oid);
> +					oc->mode = ce->ce_mode;
> +				}
>  				free(new_path);
> -				if (reject_tree_in_index(repo, only_to_die, ce,
> -							 stage, prefix, cp))
> -					return -1;
> -				oidcpy(oid, &ce->oid);
> -				oc->mode = ce->ce_mode;
> -				return 0;
> +				return ret;
>  			}

... the fix matches exactly what anybody would expect from the
problem description, i.e., "Do not free new_path before we are done
with using cp".

Will queue.  Thanks.