Hi Aaron,
On Mon, Jul 20, 2026 at 10:34:58PM -0400, Aaron Merey wrote:
> On Fri, Jul 10, 2026 at 6:35 PM Mark Wielaard <[email protected]> wrote:
> > static Dwarf *
> > check_section (Dwarf *result, size_t shstrndx, Elf_Scn *scn, bool inscngrp)
> > @@ -190,11 +193,11 @@ check_section (Dwarf *result, size_t shstrndx,
> > Elf_Scn *scn, bool inscngrp)
> > {
> > /* .debug_cu_index and .debug_tu_index don't have a .dwo suffix,
> > but they are for DWO. */
> > - if (result->type != TYPE_DWO
> > + if (result->type != DWARF_T_DWO
> > && (cnt == IDX_debug_cu_index || cnt == IDX_debug_tu_index))
> > continue;
> > bool need_dot_dwo =
> > - (result->type == TYPE_DWO
> > + (result->type == DWARF_T_DWO
> > && cnt != IDX_debug_cu_index
> > && cnt != IDX_debug_tu_index);
>
> Should IDX_debug_dwp be included here since it also doesn't have a .dwo
> suffix?
Yes, well spotted. It can be part of either a dwp DWARF_T_DWO or
DWARF_T_PLAIN, but in both cases it won't have a .dwo extension.
debug_sup and gnu_debugaltlink however cannot be part of DWARF_T_DWO.
> > @@ -545,24 +576,58 @@ scngrp_read (Dwarf *result, Elf *elf, size_t
> > shstrndx, Elf_Scn *scngrp)
> > Elf32_Word *scnidx = (Elf32_Word *) data->d_buf;
> > size_t cnt;
> >
> > - /* First check the type (PLAIN, DWO, LTO) we are looking for. We
> > - prefer PLAIN if available over DWO, over LTO. */
> > - for (cnt = 1; cnt * sizeof (Elf32_Word) <= data->d_size; ++cnt)
> > + if (requested_type == DWARF_T_AUTO)
> > {
> > - Elf_Scn *scn = elf_getscn (elf, scnidx[cnt]);
> > - if (scn == NULL)
> > + /* First check the type (PLAIN, DWO, LTO) we are looking for. We
> > + prefer PLAIN if available over DWO, over LTO. */
> > + for (cnt = 1; cnt * sizeof (Elf32_Word) <= data->d_size; ++cnt)
> > {
> > - /* A section group refers to a non-existing section. Should
> > - never happen. */
> > + Elf_Scn *scn = elf_getscn (elf, scnidx[cnt]);
> > + if (scn == NULL)
> > + {
> > + /* A section group refers to a non-existing section. Should
> > + never happen. */
> > + Dwarf_Sig8_Hash_free (&result->sig8_hash);
> > + __libdw_seterrno (DWARF_E_INVALID_ELF);
> > + free (result);
> > + return NULL;
> > + }
> > +
> > + Dwarf_Type type = scn_dwarf_type (result, shstrndx, scn);
> > + if (type > result->type)
> > + result->type = type;
> > + }
> > + }
> > + else
> > + {
> > + /* Check there is at least one section of the requested type. */
> > + bool found = false;
> > + for (cnt = 1; cnt * sizeof (Elf32_Word) <= data->d_size && !found;
> > ++cnt)
> > + {
> > + Elf_Scn *scn = elf_getscn (elf, scnidx[cnt]);
> > + if (scn == NULL)
> > + {
> > + Dwarf_Sig8_Hash_free (&result->sig8_hash);
> > + __libdw_seterrno (DWARF_E_INVALID_ELF);
> > + free (result);
> > + return NULL;
> > + }
> > +
> > + Dwarf_Type type = scn_dwarf_type (result, shstrndx, scn);
> > + if (type == requested_type)
> > + found = true;
>
> There is an assert around line 636 in dwarf_begin_elf.c that will fail
> if a malformed binary has scngrp containing non-existing sections.
Yes, at the end of scngrp_read.
> The loop in the `if (requested_type == DWARF_T_AUTO)` branch verifies
> every scngrp entry corresponds to a real section but the new else branch
> loop will stop early if found is set to true. In this case not every
> entry is verified.
>
> It might be worth addressing this to prevent another security bug report.
> We can still exit the loop early but replace the assert with an early
> return.
Yes, good idea. In general asserts are tricky in a library unless you
are really, really sure they can never trigger.
I'll sent a new version.
Thanks,
Mark
