Hi Aaron, Hi Constantine, On Thu, 2025-04-10 at 10:52 -0400, Aaron Merey wrote: > handle_dynamic_symtab can attempt to read symbol and version data from > file offset 0 if the associated DT_ tags aren't found. > > Fix this by only reading symbol and version data when non-zero file > offsets have been found. > > https://sourceware.org/bugzilla/show_bug.cgi?id=32864
So I think the basic observation that elf_getdata_rawchunk happily returns a chunk of the ELF file starting at offset zero (which is basically never what you want) is correct. Given that symdata, symstrdata, versym_data, verdef_data and verneed_data are initialized to NULL, checking for the offs index to not be zero before trying to call elf_getdata_rawchunk makes sense. It is a good update. But note that the code itself isn't very robust against bad input. Both verneed_data->d_buf and verdef_data->d_buf are used without checking whether verneed_data or verdef_data is NULL. At least against bad input data. It looks to me as if addrs[i_verdefnum] (DT_VERDEFNUM) and addrs[i_verneednum] (DT_VERNEEDNUM) could be set without offs[i_verneed] or offs[iverdef] being set. So maybe explicitly also check that (or file a bug report to check that in the future)? Thanks, Mark > Suggested-by: Constantine Bytensky <cbyten...@gmail.com> > Signed-off-by: Aaron Merey <ame...@redhat.com> > --- > src/readelf.c | 50 +++++++++++++++++++++++++++++++------------------- > 1 file changed, 31 insertions(+), 19 deletions(-) > > diff --git a/src/readelf.c b/src/readelf.c > index 5b0e7b47..0452977f 100644 > --- a/src/readelf.c > +++ b/src/readelf.c > @@ -3047,18 +3047,25 @@ handle_dynamic_symtab (Ebl *ebl) > Elf_Data *verdef_data = NULL; > Elf_Data *verneed_data = NULL; > > - symdata = elf_getdata_rawchunk ( > - ebl->elf, offs[i_symtab], > - gelf_fsize (ebl->elf, ELF_T_SYM, syments, EV_CURRENT), ELF_T_SYM); > - symstrdata = elf_getdata_rawchunk (ebl->elf, offs[i_strtab], > addrs[i_strsz], > - ELF_T_BYTE); > - versym_data = elf_getdata_rawchunk ( > - ebl->elf, offs[i_versym], syments * sizeof (Elf64_Half), ELF_T_HALF); > + if (offs[i_symtab] != 0) > + symdata = elf_getdata_rawchunk ( > + ebl->elf, offs[i_symtab], > + gelf_fsize (ebl->elf, ELF_T_SYM, syments, EV_CURRENT), ELF_T_SYM); > + > + if (offs[i_strtab] != 0) > + symstrdata = elf_getdata_rawchunk (ebl->elf, offs[i_strtab], > addrs[i_strsz], > + ELF_T_BYTE); > + > + if (offs[i_versym] != 0) > + versym_data = elf_getdata_rawchunk ( > + ebl->elf, offs[i_versym], syments * sizeof (Elf64_Half), ELF_T_HALF); > > /* Get the verneed_data without vernaux. */ > - verneed_data = elf_getdata_rawchunk ( > - ebl->elf, offs[i_verneed], addrs[i_verneednum] * sizeof > (Elf64_Verneed), > - ELF_T_VNEED); > + if (offs[i_verneed] != 0) > + verneed_data = elf_getdata_rawchunk ( > + ebl->elf, offs[i_verneed], addrs[i_verneednum] * sizeof (Elf64_Verneed), > + ELF_T_VNEED); > + > size_t vernauxnum = 0; > size_t vn_next_offset = 0; > > @@ -3071,14 +3078,18 @@ handle_dynamic_symtab (Ebl *ebl) > } > > /* Update the verneed_data to include the vernaux. */ > - verneed_data = elf_getdata_rawchunk ( > - ebl->elf, offs[i_verneed], > - (addrs[i_verneednum] + vernauxnum) * sizeof (GElf_Verneed), > ELF_T_VNEED); > + if (offs[i_verneed] != 0) > + verneed_data = elf_getdata_rawchunk ( > + ebl->elf, offs[i_verneed], > + (addrs[i_verneednum] + vernauxnum) * sizeof (GElf_Verneed), > + ELF_T_VNEED); > > /* Get the verdef_data without verdaux. */ > - verdef_data = elf_getdata_rawchunk ( > - ebl->elf, offs[i_verdef], addrs[i_verdefnum] * sizeof (Elf64_Verdef), > - ELF_T_VDEF); > + if (offs[i_verdef] != 0) > + verdef_data = elf_getdata_rawchunk ( > + ebl->elf, offs[i_verdef], addrs[i_verdefnum] * sizeof (Elf64_Verdef), > + ELF_T_VDEF); > + > size_t verdauxnum = 0; > size_t vd_next_offset = 0; > > @@ -3091,9 +3102,10 @@ handle_dynamic_symtab (Ebl *ebl) > } > > /* Update the verdef_data to include the verdaux. */ > - verdef_data = elf_getdata_rawchunk ( > - ebl->elf, offs[i_verdef], > - (addrs[i_verdefnum] + verdauxnum) * sizeof (GElf_Verdef), ELF_T_VDEF); > + if (offs[i_verdef] != 0) > + verdef_data = elf_getdata_rawchunk ( > + ebl->elf, offs[i_verdef], > + (addrs[i_verdefnum] + verdauxnum) * sizeof (GElf_Verdef), ELF_T_VDEF); > > unsigned int nsyms = (unsigned int)syments; > process_symtab (ebl, nsyms, 0, 0, 0, symdata, versym_data, symstrdata,
