From: Omar Polo <op@omarpolo.com>
Subject: Re: misc tweaks for privsep.c
To: Stefan Sperling <stsp@stsp.name>
Cc: gameoftrees@openbsd.org
Date: Tue, 14 Jun 2022 15:01:02 +0200

Stefan Sperling <stsp@stsp.name> wrote:
> On Tue, Jun 14, 2022 at 11:59:22AM +0200, Omar Polo wrote:
> > blob - a63073982fd405c8f986e41f79764f7b0f74ee7c
> > file + lib/privsep.c
> > --- lib/privsep.c
> > +++ lib/privsep.c
> > @@ -1116,6 +1101,11 @@ got_privsep_recv_index_progress(int *done, int *nobj_t
> >  			break;
> >  		}
> >  		iprogress = (struct got_imsg_index_pack_progress *)imsg.data;
> > +		if (iprogress->nobj_total < 0 || iprogress->nobj_indexed < 0 ||
> > +		    iprogress->nobj_loose < 0 || iprogress->nobj_resolved < 0) {
> > +			err = got_error(GOT_ERR_PRIVSEP_LEN);
> > +			break;
> > +		}
> 
> Could this be using GOT_ERR_RANGE instead?

actually i was a bit unsure of GOT_ERR_PRIVSEP_ERR here, thanks for
clarifying.

> PRIVSEP_LEN refers to the length of the entire imsg, not values
> reported within the message. It's fine to use PRIVPSEN_LEN where
> a value describes an amount of records that should be present
> in the message, but this is not the case here.
> 
> > @@ -1148,6 +1138,9 @@ got_privsep_get_imsg_obj(struct got_object **obj, stru
> >  		return got_error(GOT_ERR_PRIVSEP_LEN);
> >  	iobj = imsg->data;
> >  
> > +	if (iobj->pack_offset < 0)
> > +		return got_error(GOT_ERR_PRIVSEP_LEN);
> 
> As above, except GOT_ERR_PACK_OFFSET might also be a valid choice here.

will do, thanks!