Hi Matej,
On Fri, Jul 03, 2026 at 05:20:55PM +0200, Matej Smycka wrote:
> 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.
Right pread_retry is unable to read "beyond" the end of the fd.
> 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.
Ah, missed that in the review. Thanks for catching that.
> 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.
And lets add that to the testcase:
https://sourceware.org/cgit/elfutils/commit/?id=7dacdb1ddb46
> 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;
Looks good. Pushed as:
https://sourceware.org/cgit/elfutils/commit/?id=3a122a068285
Thanks,
Mark
