Hi Aaron,
On Fri, Jan 24, 2025 at 08:32:48PM -0500, Aaron Merey wrote:
> debuginfod_validate_imasig might call free on an uninitialized sig_buf
> due to a goto that can occur before sig_buf is set to NULL.
>
> Fix this by setting sig_buf to NULL before the goto.
The first thing after exit_validate is free (sig_buf) and the goto
does indeed jump over the initialization of sig_buf. So this change looks
correct to me.
I am surprised gcc didn't warn about this. Maybe in practice all local
variables with initializers get initialized together at the start of
the function?
But relying on that seems wrong (and confusing), so please commit this
cleanup.
Thanks,
Mark
> Signed-off-by: Aaron Merey <ame...@redhat.com>
> ---
> debuginfod/debuginfod-client.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
> index deff19ff..d89beae9 100644
> --- a/debuginfod/debuginfod-client.c
> +++ b/debuginfod/debuginfod-client.c
> @@ -1587,6 +1587,7 @@ debuginfod_validate_imasig (debuginfod_client *c, int
> fd)
> {
> int rc = ENOSYS;
>
> + char* sig_buf = NULL;
> EVP_MD_CTX *ctx = NULL;
> if (!c || !c->winning_headers)
> {
> @@ -1594,7 +1595,6 @@ debuginfod_validate_imasig (debuginfod_client *c, int
> fd)
> goto exit_validate;
> }
> // Extract the HEX IMA-signature from the header
> - char* sig_buf = NULL;
> char* hdr_ima_sig = strcasestr(c->winning_headers,
> "x-debuginfod-imasignature");
> if (!hdr_ima_sig || 1 != sscanf(hdr_ima_sig +
> strlen("x-debuginfod-imasignature:"), "%ms", &sig_buf))
> {
> --
> 2.47.1
>
