From: Stefan Sperling <stsp@stsp.name>
Subject: Re: ID metadata leak
To: Mark Jamsek <mark@jamsek.com>
Cc: Kyle Ackerman <kackerman0102@gmail.com>, gameoftrees@openbsd.org
Date: Tue, 12 Nov 2024 21:41:17 +0100

On Wed, Nov 13, 2024 at 03:00:42AM +1100, Mark Jamsek wrote:
> I think there's still a leak in got_send_pack() when the reference is a
> duplicate and thus not inserted into the `have_refs` list in the call to
> insert_sendable_ref(). Specifically, the `s` argument passed as the
> target_refname parameter, which points to an object obtained by either
> strdup(3) or asprintf(3), is leaked in the duplicate case. It is
> unconditionally set to NULL on each iteration with the assumption it
> will be released in the subsequent got_pathlist_free() call.
> 
> Similarly, got_fetch_pack() still leaks `refname`, which is also the
> argument passed as the path parameter to got_pathlist_insert(). Like
> `id`, it is allocated in got_privsep_recv_fetch_progress(), but it is
> not released in the duplicate case like we now do with `id`. Relatedly,
> whenever the got_pathlist_insert() call in got_fetch_pack() fails, the
> path entry is not inserted into the refs list and we jump straight to
> done:, so both refname and id are leaked.
> 
> The first of the below diffs plugs these leaks. The second is a small,
> nearby whitespace fix spotted while investigating this.
> 

Makes sense, thank you! ok by me.