read_number_entries () reads the archive symbol index entry count. On
the mmap path it copied w (4 or 8) bytes from the mapping without
checking that those bytes are present:
if (elf->map_address != NULL)
memcpy (&u, elf->map_address + *offp, w);
else if ((size_t) pread_retry (elf->fildes, &u, w, *offp) != w)
return -1;
The pread path rejects a short read, but the mmap path does not. The
only preceding size check (SARMAG + sizeof (struct ar_hdr) >
maximum_size) covers the 68-byte index-member header; the count field
sits immediately after it and index_size (the member's real size) is
only validated later. A mmap'd archive truncated to exactly the index
header (a "/" member with no content) therefore passes the header check,
and read_number_entries () reads w bytes past the end of the mapping.
This is the same defect addressed for the over-wide read in commit
8b192160 ("libelf: fix OOB read in elf_getarsym 32-bit archive index
count"), which narrowed the read from 8 to w bytes but did not add a
bounds check, so a sufficiently truncated archive still over-reads.
Reject the read when the count field lies outside the mapping, matching
the guarantee the pread path already provides.
Reproduced with a guard page: a 68-byte header-only archive built with
elf_memory () and placed at the end of a mapping with an inaccessible
page following it crashes on an unfixed build and returns
ELF_E_NO_INDEX on a fixed build.
Signed-off-by: Matej Smycka <[email protected]>
---
libelf/elf_getarsym.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/libelf/elf_getarsym.c b/libelf/elf_getarsym.c
index 346596d7..79aa3bbe 100644
--- a/libelf/elf_getarsym.c
+++ b/libelf/elf_getarsym.c
@@ -53,9 +53,13 @@ read_number_entries (uint64_t *nump, Elf *elf, size_t *offp,
bool index64_p)
size_t w = index64_p ? 8 : 4;
if (elf->map_address != NULL)
- /* Use memcpy instead of pointer dereference so as not to assume the
- field is naturally aligned within the file. */
- memcpy (&u, elf->map_address + *offp, w);
+ {
+ if (*offp + w > elf->start_offset + elf->maximum_size)
+ return -1;
+ /* Use memcpy instead of pointer dereference so as not to assume the
+ field is naturally aligned within the file. */
+ memcpy (&u, elf->map_address + *offp, w);
+ }
else if ((size_t) pread_retry (elf->fildes, &u, w, *offp) != w)
return -1;
--
2.47.3
