"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
Re: fix overzelous sanity check in got_privsep_get_imsg_obj
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Sat, 18 Jun 2022 16:17:15 +0200

Download raw body.

Thread
Omar Polo <op@omarpolo.com> 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 <op@omarpolo.com>
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;