From: Omar Polo <op@omarpolo.com>
Subject: Re: plug memory leak in got_pack_dump_delt_chain_to_file()
To: "Todd C. Miller" <millert@openbsd.org>
Cc: gameoftrees@openbsd.org
Date: Tue, 13 Feb 2024 20:57:52 +0100

On 2024/02/13 12:51:44 -0700, Todd C. Miller <millert@openbsd.org> wrote:
> Won't this cause the uninitialized contents of accum_buf to be
> written out?  I think this can happen for other error paths as well.

ah yeah, I forgot to ask about this.  accum_size is still zero so we
don't end up reading the uninitialized memory.  However, we may clobber
err on fwrite, and we could handle it slightly better.

what about this?

diff -s /home/op/w/got
commit - 8c8d22ba3f20c7468ada36f3f21837de6483fa8c
path + /home/op/w/got (staged changes)
blob - c5c1be69a232bf7e5a17ad4a0e06d81efbc1d8f8
blob + 2bb13ec800c4d056689ef15d9179ab3d8151a359
--- lib/pack.c
+++ lib/pack.c
@@ -1643,12 +1643,16 @@ got_pack_dump_delta_chain_to_file(size_t *result_size,
 			}
 		}
 	}
 
 done:
 	free(base_buf);
+	if (err) {
+		free(accum_buf);
+		accum_buf = NULL;
+	}
 	if (accum_buf) {
 		size_t len = fwrite(accum_buf, 1, accum_size, outfile);
 		free(accum_buf);
 		if (len != accum_size)
 			err = got_ferror(outfile, GOT_ERR_IO);
 	}