libelf: Make sure string returned by elf_strptr is NUL terminated.

The result of elf_strptr is often used directly to print or strcmp
the string. If the section data was truncated or corrupted that could
lead to invalid memory reads possibly crashing the application.
https://bugzilla.redhat.com/show_bug.cgi?id=1170810#c24

Reported-by: Alexander Cherepanov <cherepan@mccme.ru>
Signed-off-by: Mark Wielaard <mjw@redhat.com>

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 42d2f0ffc1e924cf1c549df19868b64ede017cfe..f175630189c778341f318dee0adc16a0c54cc369 100644 (file)
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-22  Mark Wielaard  <mjw@redhat.com>
+
+       * elf_strptr (elf_strptr): Make sure returned string is NUL
+       terminated.
+
 2015-01-21  Mark Wielaard  <mjw@redhat.com>
 
        * elf_strptr.c (elf_strptr): Check data_list_rear == NULL instead

diff --git a/libelf/elf_strptr.c b/libelf/elf_strptr.c
index 62936a0f7f609915466eef92900a95f524925c2a..e73bf3605fb2423a1e797e7e3905640e0f0b4c08 100644 (file)
--- a/libelf/elf_strptr.c
+++ b/libelf/elf_strptr.c
@@ -86,6 +86,7 @@ elf_strptr (elf, idx, offset)
        }
     }
 
+  size_t sh_size = 0;
   if (elf->class == ELFCLASS32)
     {
       Elf32_Shdr *shdr = strscn->shdr.e32 ?: __elf32_getshdr_rdlock (strscn);
@@ -96,6 +97,7 @@ elf_strptr (elf, idx, offset)
          goto out;
        }
 
+      sh_size = shdr->sh_size;
       if (unlikely (offset >= shdr->sh_size))
        {
          /* The given offset is too big, it is beyond this section.  */
@@ -113,6 +115,7 @@ elf_strptr (elf, idx, offset)
          goto out;
        }
 
+      sh_size = shdr->sh_size;
       if (unlikely (offset >= shdr->sh_size))
        {
          /* The given offset is too big, it is beyond this section.  */
@@ -141,7 +144,14 @@ elf_strptr (elf, idx, offset)
       // rawdata_base can be set while rawdata.d hasn't been
       // initialized yet (when data_read is zero). So we cannot just
       // look at the rawdata.d.d_size.
-      result = &strscn->rawdata_base[offset];
+
+      /* Make sure the string is NUL terminated.  Start from the end,
+        which very likely is a NUL char.  */
+      if (likely (memrchr (&strscn->rawdata_base[offset],
+                         '\0', sh_size - offset) != NULL))
+       result = &strscn->rawdata_base[offset];
+      else
+       __libelf_seterrno (ELF_E_INVALID_INDEX);
     }
   else
     {
@@ -153,7 +163,16 @@ elf_strptr (elf, idx, offset)
          if (offset >= (size_t) dl->data.d.d_off
              && offset < dl->data.d.d_off + dl->data.d.d_size)
            {
-             result = (char *) dl->data.d.d_buf + (offset - dl->data.d.d_off);
+             /* Make sure the string is NUL terminated.  Start from
+                the end, which very likely is a NUL char.  */
+             if (likely (memrchr ((char *) dl->data.d.d_buf
+                                  + (offset - dl->data.d.d_off), '\0',
+                                  (dl->data.d.d_size
+                                   - (offset - dl->data.d.d_off))) != NULL))
+               result = ((char *) dl->data.d.d_buf
+                         + (offset - dl->data.d.d_off));
+             else
+               __libelf_seterrno (ELF_E_INVALID_INDEX);
              break;
            }