Hi Mark,

On Wed, May 13, 2026 at 2:42 PM Mark Wielaard <[email protected]> wrote:
>
> Double check file size the server sent against the file size we
> actually got in debuginfod_validate_imasig. So we check the signature
> over the whole file size as we received it. Otherwise we might be
> creating a hash over a shorter (possibly zero sized) data. This makes
> sure the server sents a signature that should match the full file (and
> not just an arbitrary shorter prefix).
>
>         * debuginfod/debuginfod-client.c (debuginfod_validate_imasig):
>         Call fstat on fd and check x-debuginfod-size equals received
>         file size.
>
> Signed-off-by: Mark Wielaard <[email protected]>
> ---
>  debuginfod/debuginfod-client.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-client.c
> index 6340c8c27a4f..3dcada612de9 100644
> --- a/debuginfod/debuginfod-client.c
> +++ b/debuginfod/debuginfod-client.c
> @@ -1640,6 +1640,21 @@ debuginfod_validate_imasig (debuginfod_client *c, int 
> fd)
>        goto exit_validate;
>      }
>
> +    /* Don't trust the size the server sent us, double check against the
> +       file size that we actually got.  That way we calculate the hash
> +       over the whole file and not a shorter (possibly empty) data size.  */
> +    struct stat st;
> +    if (fstat (fd, &st) == -1)
> +    {
> +      rc = -errno;
> +      goto exit_validate;
> +    }
> +    if (data_len != st.st_size)
> +    {
> +      rc = -EINVAL;
> +      goto exit_validate;

It might be worth logging the reason for this error to verbose_fd, if
it's set. Also this function's docstring says -EINVAL indicates
"signature invalidity". This size mismatch is arguably a kind of
signature invalidity, but OTOH we could use a different errno here
(-EBADMSG?).

Aaron

> +    }
> +
>      char file_data[DATA_SIZE]; // imaevm.h data chunk hash size
>      ssize_t n;
>      for(off_t k = 0; k < data_len; k += n)
> --
> 2.53.0
>

Reply via email to