libelf: gelf_getphdr should check phdr index is valid.

elf_getphdrnum does checks the phdrnum makes sense. But gelf_getphdr
checked the given index against the "raw" e_phnum or internal
__elf_getphdrnum_rdlock result without checking. Extract the checking
code into a new internal  __elf_getphdrnum_chk_rdlock function and
use that.

Found by afl-fuzz.

Signed-off-by: Mark Wielaard <mjw@redhat.com>

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index f2b3f2151d727daaac5199fadcbe444cc8e1a0fc..2ca95092636bd9a83865d06ead42f99826eda0fd 100644 (file)
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,11 @@
+2014-12-30  Mark Wielaard  <mjw@redhat.com>
+
+       * elf_getphdrnum.c (__elf_getphdrnum_chk_rdlock): New function.
+       (elf_getphdrnum): Call __elf_getphdrnum_chk_rdlock.
+       * gelf_getphdr (gelf_getphdr): Call __elf_getphdrnum_chk_rdlock
+       and always check ndx against phnum.
+       * libelfP.h (__elf_getphdrnum_chk_rdlock): New internal function.
+
 2014-12-25  Mark Wielaard  <mjw@redhat.com>
 
        * elf_begin.c (__libelf_next_arhdr_wrlock): ar_size cannot be

diff --git a/libelf/elf_getphdrnum.c b/libelf/elf_getphdrnum.c
index 63c27fb1dbef9af71ec4a5e74df8f0b54dbfa6a7..f2fad87a4e37b5b086202078405b196ebc44169a 100644 (file)
--- a/libelf/elf_getphdrnum.c
+++ b/libelf/elf_getphdrnum.c
@@ -80,23 +80,11 @@ __elf_getphdrnum_rdlock (elf, dst)
 }
 
 int
-elf_getphdrnum (elf, dst)
+__elf_getphdrnum_chk_rdlock (elf, dst)
      Elf *elf;
      size_t *dst;
 {
-  int result;
-
-  if (elf == NULL)
-    return -1;
-
-  if (unlikely (elf->kind != ELF_K_ELF))
-    {
-      __libelf_seterrno (ELF_E_INVALID_HANDLE);
-      return -1;
-    }
-
-  rwlock_rdlock (elf->lock);
-  result = __elf_getphdrnum_rdlock (elf, dst);
+  int result = __elf_getphdrnum_rdlock (elf, dst);
 
   /* Do some sanity checking to make sure phnum and phoff are consistent.  */
   Elf64_Off off = (elf->class == ELFCLASS32
@@ -105,14 +93,13 @@ elf_getphdrnum (elf, dst)
   if (unlikely (off == 0))
     {
       *dst = 0;
-      goto out;
+      return result;
     }
 
   if (unlikely (off >= elf->maximum_size))
     {
       __libelf_seterrno (ELF_E_INVALID_DATA);
-      result = -1;
-      goto out;
+      return -1;
     }
 
   /* Check for too many sections.  */
@@ -121,15 +108,34 @@ elf_getphdrnum (elf, dst)
   if (unlikely (*dst > SIZE_MAX / phdr_size))
     {
       __libelf_seterrno (ELF_E_INVALID_DATA);
-      result = -1;
-      goto out;
+      return -1;
     }
 
   /* Truncated file?  Don't return more than can be indexed.  */
   if (unlikely (elf->maximum_size - off < *dst * phdr_size))
     *dst = (elf->maximum_size - off) / phdr_size;
 
-out:
+  return result;
+}
+
+int
+elf_getphdrnum (elf, dst)
+     Elf *elf;
+     size_t *dst;
+{
+  int result;
+
+  if (elf == NULL)
+    return -1;
+
+  if (unlikely (elf->kind != ELF_K_ELF))
+    {
+      __libelf_seterrno (ELF_E_INVALID_HANDLE);
+      return -1;
+    }
+
+  rwlock_rdlock (elf->lock);
+  result = __elf_getphdrnum_chk_rdlock (elf, dst);
   rwlock_unlock (elf->lock);
 
   return result;

diff --git a/libelf/gelf_getphdr.c b/libelf/gelf_getphdr.c
index 3bf7123f7acf485bf67e730a89018d5638ea2df0..1a6ee62fcce4a0f9142eccb22730d459f5ca3021 100644 (file)
--- a/libelf/gelf_getphdr.c
+++ b/libelf/gelf_getphdr.c
@@ -80,10 +80,8 @@ gelf_getphdr (elf, ndx, dst)
 
       /* Test whether the index is ok.  */
       size_t phnum;
-      if (ndx >= elf->state.elf32.ehdr->e_phnum
-         && (elf->state.elf32.ehdr->e_phnum != PN_XNUM
-             || __elf_getphdrnum_rdlock (elf, &phnum) != 0
-             || (size_t) ndx >= phnum))
+      if (__elf_getphdrnum_chk_rdlock (elf, &phnum) != 0
+         || (size_t) ndx >= phnum)
        {
          __libelf_seterrno (ELF_E_INVALID_INDEX);
          goto out;
@@ -122,10 +120,8 @@ gelf_getphdr (elf, ndx, dst)
 
       /* Test whether the index is ok.  */
       size_t phnum;
-      if (ndx >= elf->state.elf64.ehdr->e_phnum
-         && (elf->state.elf64.ehdr->e_phnum != PN_XNUM
-             || __elf_getphdrnum_rdlock (elf, &phnum) != 0
-             || (size_t) ndx >= phnum))
+      if (__elf_getphdrnum_chk_rdlock (elf, &phnum) != 0
+         || (size_t) ndx >= phnum)
        {
          __libelf_seterrno (ELF_E_INVALID_INDEX);
          goto out;

diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index 52cf7457df25baa2cae1b626954599c18bc83712..3b24e75c298097e462da60e7750abe25c049d9c5 100644 (file)
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -511,6 +511,8 @@ extern Elf_Scn *__elf64_offscn_internal (Elf *__elf, Elf64_Off __offset)
      attribute_hidden;
 extern int __elf_getphdrnum_rdlock (Elf *__elf, size_t *__dst)
      internal_function;
+extern int __elf_getphdrnum_chk_rdlock (Elf *__elf, size_t *__dst)
+     internal_function;
 extern int __elf_getshdrnum_rdlock (Elf *__elf, size_t *__dst)
      internal_function;
 extern int __elf_getshdrstrndx_internal (Elf *__elf, size_t *__dst)