Hi Mark,
On Sun, Sep 13, 2026 at 6:09 PM Mark Wielaard <[email protected]> wrote:
>
> And a couple of sanity checks and make sure that memory is released in
> all error paths.
>
> * elf-from-memory.c (elf_from_remote_memory): Check shentsize
> is sane (or zero). Make sure shdrs_end calculation uses 64bit
> unsigned values and check for overflow. Make sure nread >=
> sizeof (Elf64_Ehdr) for ELFCLASS64. free buffer and phdrsp on
> libelf_error. Check segment_end for overflow. Make sure
> PT_LOAD end didn't wrap around. Directly go to libelf_error
> when elf_memory fails.
>
> Signed-off-by: Mark Wielaard <[email protected]>
LGTM.
Aaron
> ---
> libdwfl/elf-from-memory.c | 33 +++++++++++++++++++++++++++------
> 1 file changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/libdwfl/elf-from-memory.c b/libdwfl/elf-from-memory.c
> index 4015efd23427..c6137f73f14d 100644
> --- a/libdwfl/elf-from-memory.c
> +++ b/libdwfl/elf-from-memory.c
> @@ -77,6 +77,7 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
> return NULL;
> }
>
> + /* Note minread is just enough for an Elf32_Ehdr. */
> ssize_t nread = (*read_memory) (arg, buffer, ehdr_vma,
> sizeof (Elf32_Ehdr), initial_bufsize);
> if (nread <= 0)
> @@ -130,6 +131,8 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
> if (elf32_xlatetom (&xlateto, &xlatefrom, buffer[EI_DATA]) == NULL)
> {
> libelf_error:
> + free (buffer);
> + free (phdrsp);
> __libdwfl_seterrno (DWFL_E_LIBELF);
> return NULL;
> }
> @@ -143,10 +146,17 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
> zero sh_size field. We ignore this here because getting shdrs
> is just a nice bonus (see below where we trim the last phdrs
> PT_LOAD segment). */
> - shdrs_end = ehdr.e32.e_shoff + ehdr.e32.e_shnum * ehdr.e32.e_shentsize;
> + GElf_Xword shnum32 = ehdr.e32.e_shnum;
> + GElf_Xword shentsize32 = ehdr.e32.e_shentsize;
> + if (shentsize32 != sizeof (Elf32_Shdr) && shentsize32 != 0)
> + goto bad_elf;
> + shdrs_end = ehdr.e32.e_shoff + shnum32 * shentsize32;
> break;
>
> case ELFCLASS64:
> + /* Unlikely, but minread was for just an Elf32_Ehdr. */
> + if (unlikely ((size_t) nread < sizeof (Elf64_Ehdr)))
> + goto bad_elf;
> xlatefrom.d_size = sizeof (Elf64_Ehdr);
> if (elf64_xlatetom (&xlateto, &xlatefrom, buffer[EI_DATA]) == NULL)
> goto libelf_error;
> @@ -156,7 +166,11 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
> if (phentsize != sizeof (Elf64_Phdr) || phnum == 0)
> goto bad_elf;
> /* See the NOTE above for shdrs_end and ehdr.e32.e_shnum. */
> - shdrs_end = ehdr.e64.e_shoff + ehdr.e64.e_shnum * ehdr.e64.e_shentsize;
> + GElf_Xword shnum64 = ehdr.e64.e_shnum;
> + GElf_Xword shentsize64 = ehdr.e64.e_shentsize;
> + if (shentsize64 != sizeof (Elf64_Shdr) && shentsize64 != 0)
> + goto bad_elf;
> + shdrs_end = ehdr.e64.e_shoff + shnum64 * shentsize64;
> break;
>
> default:
> @@ -254,6 +268,11 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
> GElf_Off segment_end = ((offset + filesz + pagesize - 1)
> & -pagesize);
>
> + /* Check for overflow or too big segment size. */
> + if (unlikely (segment_end < (offset & -pagesize)
> + || segment_end > SSIZE_MAX))
> + goto bad_elf;
> +
> if (segment_end > (GElf_Off) contents_size)
> contents_size = segment_end;
>
> @@ -310,6 +329,11 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>
> GElf_Off start = offset & -pagesize;
> GElf_Off end = (offset + filesz + pagesize - 1) & -pagesize;
> +
> + /* Make sure end didn't wrap around. */
> + if (unlikely (end < start))
> + goto bad_elf;
> +
> /* The final contents_size is the trimmed last segment's end, which
> may be smaller than an earlier segment's start (segments_end above
> tracks the last PT_LOAD, not the maximum). Skip any segment that
> @@ -373,10 +397,7 @@ elf_from_remote_memory (GElf_Addr ehdr_vma,
>
> Elf *elf = elf_memory ((char *) buffer, contents_size);
> if (elf == NULL)
> - {
> - free (buffer);
> - goto libelf_error;
> - }
> + goto libelf_error;
>
> elf->flags |= ELF_F_MALLOCED;
> if (loadbasep != NULL)
> --
> 2.55.0
>
