This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
[PATCH] libelf: Make sure string returned by elf_strptr is NUL terminated.
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Thu, 22 Jan 2015 12:55:43 +0100
- Subject: [PATCH] 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>
---
libelf/ChangeLog | 5 +++++
libelf/elf_strptr.c | 23 +++++++++++++++++++++--
2 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 8bf1256..e290f48 100644
--- 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 62936a0..e73bf36 100644
--- 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;
}
--
1.8.3.1