From: Omar Polo Subject: Re: fix overzelous sanity check in got_privsep_get_imsg_obj To: Omar Polo Cc: gameoftrees@openbsd.org Date: Sat, 18 Jun 2022 16:17:15 +0200 Omar Polo wrote: > It seems that i was a bit too much optimistic in c98b0f0. With got > freshly compiled from ports i sometimes get a 'bad offset in pack file' > failure. This happened in 'got status', 'got diff', ... and even in > tog, in got.git and src.git. This happens very frequently thought, but > not always. > > Curiously, this didn't happened in my local builds. > > Diff below fixes the issue for me. It moves the sanity check in only in > the case where the object was effectively packed. > > This is just fixing the behaviour thought, i don't know why/how > pack_offset was negative. i tried with > > if (obj->pack_offset < 0) > abort(); > > in got_privsep_send_obj but couldn't get it to segfault. ...because the libexec is sending good data. got_privsep_send_obj copies the data from the given struct got_object into a stack-allocated struct got_imsg_object. Except that this struct isn't zeroed and so we read garbage value when the object is not packed. so please discard my previous diff, the solution is to fix this at the sending side: ----------------------------------------------- commit 0fe55807cf233278482e51afcee4b60d5c974340 (main) from: Omar Polo date: Sat Jun 18 14:13:48 2022 UTC zero struct got_imsg_object otherwise some fields may be unitialized and fail the validation done on the receiving side. diff f2bdc381b40121c4f81d3c1fb75564dda4818cbd 0b40952057b9d91d60b6cfc756a88be33b593329 blob - 9f5f6eca6eee4371542d386c8da0e763f8bcc98a blob + 2d5319f731933605f168211ff0d2be2f135b57b2 --- lib/privsep.c +++ lib/privsep.c @@ -531,6 +531,8 @@ got_privsep_send_obj(struct imsgbuf *ibuf, struct got_ { struct got_imsg_object iobj; + memset(&iobj, 0, sizeof(iobj)); + memcpy(iobj.id, obj->id.sha1, sizeof(iobj.id)); iobj.type = obj->type; iobj.flags = obj->flags;