Re: [PATCH] Avoid divide by zero when reading invalid PDB archive

[Alan Modra <amodra@gmail.com>]

On Fri, Sep 16, 2022 at 01:40:34AM +0100, Mark Harmstone wrote:
> As discovered by Alan Modra - see
> https://sourceware.org/pipermail/binutils/2022-September/122878.html.
> 
> ---
>  bfd/pdb.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/bfd/pdb.c b/bfd/pdb.c
> index 9a431c23b1f..0ce3bc46ac6 100644
> --- a/bfd/pdb.c
> +++ b/bfd/pdb.c
> @@ -80,6 +80,12 @@ pdb_get_elt_at_index (bfd *abfd, symindex sym_index)
>  
>    block_size = bfd_getl32 (int_buf);
>  
> +  if (block_size == 0)
> +    {
> +      bfd_set_error (bfd_error_malformed_archive);
> +      return NULL;
> +    }
> +

I was looking for a little more than just preventing the divide by
zero.  Based on the doc link at llvm.org, I wrote the following:

  if ((block_size & -block_size) != block_size
      || block_size < 512
      || block_size > 4096)
    {
      bfd_set_error (bfd_error_malformed_archive);
      return NULL;
    }

But then looked further at your code and wondered about all the rest
of the 32-bit values read from file.  For example,
  block_map_addr * block_size might overflow.  (Which leads to the
question of whether you can have pdb files larger than 4G?)

You probably want something like

  file_ptr block_size, where;
  ...
  block_map_addr = bfd_getl32 (int_buf);
  if (_bfd_mul_overflow (block_map_addr, block_size, &where))
    {
      bfd_set_error (bfd_error_malformed_archive);
      return NULL;
    }
  if (bfd_seek (abfd, where, SEEK_SET))
    return NULL;

Using file_ptr for block_size will perform the multiply in the
appropriate size for the host system.

I'll commit the patch I wrote, and leave the rest for you to do at
your leisure.

-- 
Alan Modra
Australia Development Lab, IBM