From 9d481b29e773f01781f70f1bcdfc05dd66c796ea Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Thu, 22 Jan 2015 12:49:29 +0100 Subject: 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 Signed-off-by: Mark Wielaard --- libelf/ChangeLog | 5 +++++ libelf/elf_strptr.c | 23 +++++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 42d2f0ff..f1756301 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,3 +1,8 @@ +2015-01-22 Mark Wielaard + + * elf_strptr (elf_strptr): Make sure returned string is NUL + terminated. + 2015-01-21 Mark Wielaard * elf_strptr.c (elf_strptr): Check data_list_rear == NULL instead diff --git a/libelf/elf_strptr.c b/libelf/elf_strptr.c index 62936a0f..e73bf360 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; } -- cgit v1.2.3