From: Mark Wielaard <mark@klomp.org>
Date: Thu, 14 Feb 2019 10:47:59 +0000 (+0100)
Subject: libelf: Make sure ar_size is terminated when reading ar long names.
X-Git-Tag: elfutils-0.176~2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e32380ecefbb23448541367283d3b94930762986;p=thirdparty%2Felfutils.git

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 b89e93fec..d8e8fdc80 100644
--- 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 b20ab4f39..fde14c618 100644
--- 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;