From: Stefan Sperling <stsp@stsp.name>
Subject: Re: fix invalid imsg_free() in got_privsep_recv_printed_commits()
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Mon, 26 Feb 2024 17:43:39 +0100

On Mon, Feb 26, 2024 at 05:30:59PM +0100, Omar Polo wrote:
> Diff below should be safe since recv_imsg_error itself checks the
> datalen before constructing the error.

Oh, indeed! I missed this small important detail.

ok stsp@

> diff /home/op/w/got
> commit - fcece7180725bba9a781eaa892af379b1986208b
> path + /home/op/w/got
> blob - 208f38064847db1bdf2043d22f6be5691d0905c1
> file + lib/privsep.c
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -141,11 +141,6 @@ got_privsep_recv_imsg(struct imsg *imsg, struct imsgbu
>  			return got_error_from_errno("imsg_get");
>  	}
>  
> -	if (imsg->hdr.len < IMSG_HEADER_SIZE + min_datalen) {
> -		imsg_free(imsg);
> -		return got_error(GOT_ERR_PRIVSEP_LEN);
> -	}
> -
>  	if (imsg->hdr.type == GOT_IMSG_ERROR) {
>  		size_t datalen = imsg->hdr.len - IMSG_HEADER_SIZE;
>  		err = recv_imsg_error(imsg, datalen);
> @@ -153,6 +148,11 @@ got_privsep_recv_imsg(struct imsg *imsg, struct imsgbu
>  		return err;
>  	}
>  
> +	if (imsg->hdr.len < IMSG_HEADER_SIZE + min_datalen) {
> +		imsg_free(imsg);
> +		return got_error(GOT_ERR_PRIVSEP_LEN);
> +	}
> +
>  	return NULL;
>  }
>  
>