[PATCH] libelf: Check phnum overflow before multiply and remove duplicate check

[Mark Wielaard]

The size of the program header table computation might overflow. The
wrapped value was never used, but computing the wrapped product first
is unnecessary and confusing. Reorder the code so the overflow check
happens before the multiply. And remove a duplicate check under the
map_address != NULL case.

        * libelf/elf32_getphdr.c (getphdr_wrlock): Check phnum
        overflow before multiply and remove duplicate check.

Signed-off-by: Mark Wielaard <m...@klomp.org>
---
 libelf/elf32_getphdr.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/libelf/elf32_getphdr.c b/libelf/elf32_getphdr.c
index 9aa1b6259163..e41c660daf54 100644
--- a/libelf/elf32_getphdr.c
+++ b/libelf/elf32_getphdr.c
@@ -79,11 +79,18 @@ __elfw2(LIBELFBITS,getphdr_wrlock) (Elf *elf)
          goto out;
        }
 
-      /* Check this doesn't overflow.  */
+      /* Check this doesn't overflow.  Do the overflow check before
+        the multiply so size can never be a wrapped value.  */
+      if (phnum > SIZE_MAX / sizeof (ElfW2(LIBELFBITS,Phdr)))
+       {
+         __libelf_seterrno (ELF_E_INVALID_DATA);
+         goto out;
+       }
       size_t size = phnum * sizeof (ElfW2(LIBELFBITS,Phdr));
 
-      if (phnum > SIZE_MAX / sizeof (ElfW2(LIBELFBITS,Phdr))
-         || ehdr->e_phoff > elf->maximum_size
+      /* First see whether the information in the ELF header is
+        valid and it does not ask for too much.  */
+      if (ehdr->e_phoff > elf->maximum_size
          || elf->maximum_size - ehdr->e_phoff < size)
        {
          __libelf_seterrno (ELF_E_INVALID_DATA);
@@ -92,16 +99,6 @@ __elfw2(LIBELFBITS,getphdr_wrlock) (Elf *elf)
 
       if (elf->map_address != NULL)
        {
-         /* First see whether the information in the ELF header is
-            valid and it does not ask for too much.  */
-         if (unlikely (ehdr->e_phoff >= elf->maximum_size)
-             || unlikely (elf->maximum_size - ehdr->e_phoff < size))
-           {
-             /* Something is wrong.  */
-             __libelf_seterrno (ELF_E_INVALID_PHDR);
-             goto out;
-           }
-
          /* All the data is already mapped.  Use it.  */
          void *file_phdr = ((char *) elf->map_address
                             + elf->start_offset + ehdr->e_phoff);
-- 
2.56.0