debuginfod-client.c: Avoid freeing uninitialized value

Message ID 20250125013248.2694770-1-amerey@redhat.com
State Committed
Delegated to: Mark Wielaard
Headers
Series debuginfod-client.c: Avoid freeing uninitialized value |

Commit Message

Aaron Merey Jan. 25, 2025, 1:32 a.m. UTC
  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.

Signed-off-by: Aaron Merey <amerey@redhat.com>
---
 debuginfod/debuginfod-client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Mark Wielaard Jan. 25, 2025, 10:57 p.m. UTC | #1
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 <amerey@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
>
  

Patch

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))
     {