libelf: Make sure ar_size is terminated when reading ar long names.

The ar_size is given as a fixed size decimal string, right padded with
spaces.  Make sure we read it properly even if there is no terminating
space. Also sanity check len early if we can.

https://sourceware.org/bugzilla/show_bug.cgi?id=24085

Signed-off-by: Mark Wielaard <mark@klomp.org>

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index b89e93f..d8e8fdc 100644 (file)
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2019-02-14  Mark Wielaard  <mark@klomp.org>
+
+       * elf_begin.c (read_long_names): Make sure ar_size is properly
+       terminated. Sanity check len early if we can.
+
 2019-01-18  Mark Wielaard  <mark@klomp.org>
 
        * Makefile.am (INSTALL_ELFH): Add elf.h to include_HEADERS when

diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index b20ab4f..fde14c6 100644 (file)
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -736,7 +736,17 @@ read_long_names (Elf *elf)
          hdr = &hdrm;
        }
 
-      len = atol (hdr->ar_size);
+      /* The ar_size is given as a fixed size decimal string, right
+        padded with spaces.  Make sure we read it properly even if
+        there is no terminating space.  */
+      char buf[sizeof (hdr->ar_size) + 1];
+      const char *string = hdr->ar_size;
+      if (hdr->ar_size[sizeof (hdr->ar_size) - 1] != ' ')
+       {
+         *((char *) mempcpy (buf, hdr->ar_size, sizeof (hdr->ar_size))) = '\0';
+         string = buf;
+       }
+      len = atol (string);
 
       if (memcmp (hdr->ar_name, "//              ", 16) == 0)
        break;
@@ -744,6 +754,13 @@ read_long_names (Elf *elf)
       offset += sizeof (struct ar_hdr) + ((len + 1) & ~1l);
     }
 
+  /* Sanity check len early if we can.  */
+  if (elf->map_address != NULL)
+    {
+      if (len > elf->maximum_size - offset - sizeof (struct ar_hdr))
+       return NULL;
+    }
+
   /* Due to the stupid format of the long name table entry (which are not
      NUL terminted) we have to provide an appropriate representation anyhow.
      Therefore we always make a copy which has the appropriate form.  */
@@ -754,8 +771,6 @@ read_long_names (Elf *elf)
 
       if (elf->map_address != NULL)
        {
-         if (len > elf->maximum_size - offset - sizeof (struct ar_hdr))
-           goto too_much;
          /* Simply copy it over.  */
          elf->state.ar.long_names = (char *) memcpy (newp,
                                                      elf->map_address + offset
@@ -769,7 +784,6 @@ read_long_names (Elf *elf)
                                              + sizeof (struct ar_hdr))
                        != len))
            {
-           too_much:
              /* We were not able to read all data.  */
              free (newp);
              elf->state.ar.long_names = NULL;