fsck segfault (was: Re: [PATCH] revision: mark blobs needed for resolve-undo as reachable)

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Thu, Jun 09, 2022 at 04:44:20PM -0700, Junio C Hamano wrote:
> +static int fsck_resolve_undo(struct index_state *istate)
> +{
> +	struct string_list_item *item;
> +	struct string_list *resolve_undo = istate->resolve_undo;
> +
> +	if (!resolve_undo)
> +		return 0;
> +
> +	for_each_string_list_item(item, resolve_undo) {
> +		const char *path = item->string;
> +		struct resolve_undo_info *ru = item->util;
> +		int i;
> +
> +		if (!ru)
> +			continue;
> +		for (i = 0; i < 3; i++) {
> +			struct object *obj;
> +
> +			if (!ru->mode[i] || !S_ISREG(ru->mode[i]))
> +				continue;
> +
> +			obj = parse_object(the_repository, &ru->oid[i]);

parse_object() can return NULL ...

> +			if (!obj) {

... and here is the if statement to show an error in that case ...

> +				error(_("%s: invalid sha1 pointer in resolve-undo"),
> +				      oid_to_hex(&ru->oid[i]));
> +				errors_found |= ERROR_REFS;
> +			}
> +			obj->flags |= USED;

... but then there is this line which might dereference that NULL
pointer.

Perhaps all we would need is a 'continue' at the end of that 'if
(!obj)' block, or an else block for the last three statements, which
should result in the same control flow?  Dunno.

> +			fsck_put_object_name(&fsck_walk_options, &ru->oid[i],
> +					     ":(%d):%s", i, path);
> +			mark_object_reachable(obj);
> +		}
> +	}
> +	return 0;
> +}