Hi Aaron, Thanks for reviewing and merging the patch, and for taking care of the v2 cleanups in a separate commit. I'll pull the latest and confirm everything builds and the new tests pass.
Kind regards, Harshit On Mon, 19 Oct 2026 at 04:52, Aaron Merey <[email protected]> wrote: > > Hi Harshit, > > On Thu, Oct 8, 2026 at 10:24 AM Harshit Kumar <[email protected]> wrote: > > > > Three sites compute a symbol count as `sh_size / sh_entsize` or subtract > > one from it. The existing guards rejected only `sh_entsize == 0`, so a > > section that declares `sh_size < sh_entsize` truncates the quotient > > to zero and the subtraction wraps. Both fields come straight from the > > input ELF. > > > > copy_elided_sections(): > > In the symbol merge path, `stripped_nsym - 1 + unstripped_nsym - 1` wraps > > if either `stripped_nsym == 0` or `unstripped_nsym == 0`. > > When `unstripped_nsym == 0`, `total_syms` is under-allocated for > > `stripped_nsym - 2` elements and collect_symbols() writes past the end > > of the allocation. > > When `stripped_nsym == 0`, `&symbols[stripped_nsym - 1]` passes an > > underflowed pointer (&symbols[SIZE_MAX]) to collect_symbols(), which > > performs out-of-bounds heap writes. > > Additionally, the earlier section copy loop rejected only > > `shdr_mem.sh_entsize == 0`, allowing malformed SYMTAB/DYNSYM sections from > > the stripped file to propagate. > > > > add_new_section_symbols(): > > `size_t symndx_map[nsym - 1]` declared a VLA with SIZE_MAX elements when > > `nsym == 0`. Malformed symbol tables are normally rejected earlier, > > but the check in add_new_section_symbols() is retained as defense in > > depth to protect the VLA declaration against underflow. > > > > Fix the guards at all sites so the quotient can never be zero. Add a > > regression test covering malformed stripped and unstripped symbol tables. > > > > https://sourceware.org/bugzilla/show_bug.cgi?id=34705 > > > > Signed-off-by: Harshit Kumar <[email protected]> > > Thanks for the patch, it has been merged. I mistakenly merged v1 plus > the tests/Makefile.am fix. The remaining cleanups from v2 were merged > in a separate commit: > https://sourceware.org/pipermail/elfutils-devel/2026q4/009771.html > > Aaron > > > --- > > v2: > > - tests/Makefile.am: restore run-unstrip-M.sh in EXTRA_DIST. > > - tests/run-unstrip-symtab-count.sh: anchor greps on site-specific > > prefixes and use grep -q to suppress stdout. > > - tests/run-unstrip-symtab-count.sh: fix test description comments and > > copyright attribution. > > - src/unstrip.c: wrap lines exceeding 79 columns. > > - Commit message: note add_new_section_symbols() guard as defense in depth; > > rewrap message lines to <= 74 columns; remove stale line reference and > > ChangeLog block. > > > > src/unstrip.c | 27 +++-- > > tests/Makefile.am | 8 +- > > tests/run-unstrip-symtab-count.sh | 106 ++++++++++++++++++++ > > tests/testfile-unstrip-symtab0.debug.bz2 | Bin 0 -> 220 bytes > > tests/testfile-unstrip-symtab0.stripped.bz2 | Bin 0 -> 225 bytes > > tests/testfile-unstrip-symtab1.debug.bz2 | Bin 0 -> 224 bytes > > tests/testfile-unstrip-symtab1.stripped.bz2 | Bin 0 -> 179 bytes > > 7 files changed, 131 insertions(+), 10 deletions(-) > > create mode 100755 tests/run-unstrip-symtab-count.sh > > create mode 100644 tests/testfile-unstrip-symtab0.debug.bz2 > > create mode 100644 tests/testfile-unstrip-symtab0.stripped.bz2 > > create mode 100644 tests/testfile-unstrip-symtab1.debug.bz2 > > create mode 100644 tests/testfile-unstrip-symtab1.stripped.bz2 > > > > diff --git a/src/unstrip.c b/src/unstrip.c > > index 60f0916a..39d5bb65 100644 > > --- a/src/unstrip.c > > +++ b/src/unstrip.c > > @@ -638,8 +638,9 @@ add_new_section_symbols (Elf_Scn *old_symscn, size_t > > old_shnum, > > GElf_Shdr shdr_mem; > > GElf_Shdr *shdr = gelf_getshdr (symscn, &shdr_mem); > > ELF_CHECK (shdr != NULL, _("cannot get section header: %s")); > > - if (shdr->sh_entsize == 0) > > - error_exit (0, "Symbol table section cannot have zero sh_entsize"); > > + if (shdr->sh_entsize == 0 || shdr->sh_size < shdr->sh_entsize) > > + error_exit (0, _("Symbol table section cannot have zero sh_entsize" > > + " or a sh_size smaller than sh_entsize")); > > > > const size_t nsym = shdr->sh_size / shdr->sh_entsize; > > size_t symndx_map[nsym - 1]; > > @@ -1778,9 +1779,11 @@ more sections in stripped file than debug file -- > > arguments reversed?")); > > > > Elf_Data *shndxdata = NULL; /* XXX */ > > > > - if (shdr_mem.sh_entsize == 0) > > + if (shdr_mem.sh_entsize == 0 > > + || shdr_mem.sh_size < shdr_mem.sh_entsize) > > error_exit (0, > > - "SYMTAB section cannot have zero sh_entsize"); > > + _("SYMTAB section cannot have zero sh_entsize" > > + " or a sh_size smaller than sh_entsize")); > > for (size_t i = 1; i < shdr_mem.sh_size / shdr_mem.sh_entsize; > > ++i) > > { > > GElf_Sym sym_mem; > > @@ -1844,18 +1847,24 @@ more sections in stripped file than debug file -- > > arguments reversed?")); > > && bias != 0))) > > { > > /* Merge the stripped file's symbol table into the unstripped one. > > */ > > + if (stripped_symtab != NULL > > + && (stripped_symtab->shdr.sh_entsize == 0 > > + || (stripped_symtab->shdr.sh_size > > + < stripped_symtab->shdr.sh_entsize))) > > + error_exit (0, > > + _("stripped SYMTAB section cannot have zero sh_entsize" > > + " or a sh_size smaller than sh_entsize")); > > const size_t stripped_nsym = (stripped_symtab == NULL ? 1 > > : (stripped_symtab->shdr.sh_size > > - / (stripped_symtab->shdr.sh_entsize > > == 0 > > - ? 1 > > - : > > stripped_symtab->shdr.sh_entsize))); > > + / stripped_symtab->shdr.sh_entsize)); > > > > GElf_Shdr shdr_mem; > > GElf_Shdr *shdr = gelf_getshdr (unstripped_symtab, &shdr_mem); > > ELF_CHECK (shdr != NULL, _("cannot get section header: %s")); > > - if (shdr->sh_entsize == 0) > > + if (shdr->sh_entsize == 0 || shdr->sh_size < shdr->sh_entsize) > > error_exit (0, > > - "unstripped SYMTAB section cannot have zero > > sh_entsize"); > > + _("unstripped SYMTAB section cannot have zero > > sh_entsize" > > + " or a sh_size smaller than sh_entsize")); > > const size_t unstripped_nsym = shdr->sh_size / shdr->sh_entsize; > > > > /* First collect all the symbols from both tables. */ > > diff --git a/tests/Makefile.am b/tests/Makefile.am > > index dd937122..261649fd 100644 > > --- a/tests/Makefile.am > > +++ b/tests/Makefile.am > > @@ -143,7 +143,8 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh > > newfile test-nlist \ > > run-strip-reloc-ppc64.sh \ > > run-strip-nobitsalign.sh run-strip-remove-keep.sh \ > > run-unstrip-test.sh run-unstrip-test2.sh run-unstrip-test3.sh \ > > - run-unstrip-test4.sh run-unstrip-M.sh run-elfstrmerge-test.sh \ > > + run-unstrip-test4.sh run-unstrip-M.sh run-unstrip-symtab-count.sh \ > > + run-elfstrmerge-test.sh \ > > run-ecp-test.sh run-ecp-test2.sh run-alldts.sh \ > > run-elflint-test.sh run-elflint-self.sh run-ranlib-test.sh \ > > run-ranlib-test2.sh run-ranlib-test3.sh run-ranlib-test4.sh \ > > @@ -378,6 +379,11 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh > > run-ar.sh \ > > run-unstrip-test4.sh testfile-strtab.bz2 \ > > testfile-strtab.stripped.bz2 testfile-strtab.debuginfo.bz2 \ > > run-unstrip-M.sh run-elfstrmerge-test.sh \ > > + run-unstrip-symtab-count.sh \ > > + testfile-unstrip-symtab0.stripped.bz2 \ > > + testfile-unstrip-symtab0.debug.bz2 \ > > + testfile-unstrip-symtab1.stripped.bz2 \ > > + testfile-unstrip-symtab1.debug.bz2 \ > > run-elflint-self.sh run-ranlib-test.sh run-ranlib-test2.sh \ > > run-ranlib-test3.sh run-ranlib-test4.sh \ > > run-addrscopes.sh run-strings-test.sh run-funcscopes.sh \ > > diff --git a/tests/run-unstrip-symtab-count.sh > > b/tests/run-unstrip-symtab-count.sh > > new file mode 100755 > > index 00000000..0d1995b4 > > --- /dev/null > > +++ b/tests/run-unstrip-symtab-count.sh > > @@ -0,0 +1,106 @@ > > +#! /bin/sh > > +# Copyright (C) 2026 Harshit Kumar > > +# This file is part of elfutils. > > +# > > +# This file is free software; you can redistribute it and/or modify > > +# it under the terms of the GNU General Public License as published by > > +# the Free Software Foundation; either version 2 of the License, or > > +# (at your option) any later version. > > +# > > +# elfutils is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program. If not, see <http://www.gnu.org/licenses/>. > > + > > +# Regression test for the symbol count underflow in unstrip. > > +# > > +# Both files below declare a SHT_SYMTAB section whose sh_size is smaller > > +# than its sh_entsize. The quotient sh_size / sh_entsize is therefore > > zero, > > +# and the code that used to compute "symbol count minus one" wrapped. > > unstrip > > +# must reject such a file instead of doing the subtraction. > > +# > > +# symtab0: the unstripped (debug) file is the malformed one. This reaches > > +# copy_elided_sections(), where "stripped_nsym - 1 + unstripped_nsym - 1" > > +# wrapped and collect_symbols() then wrote one struct symbol past the end > > +# of the allocation. It aborted under glibc heap checking before the > > fix. > > +# > > +# symtab1: the stripped file is the malformed one, and its debug file has > > no > > +# symbol table at all. The section copy loop rejects the malformed > > symbol > > +# table from the stripped file before it can be propagated. > > +# > > +# Case 2 reuses the symtab0 files with the arguments swapped, relying on > > +# the argument order to make the malformed file the stripped one and the > > +# valid file the debug one. Without the guard on stripped_symtab, > > +# "stripped_nsym - 1" wrapped to SIZE_MAX, leading to out-of-bounds > > pointer > > +# arithmetic (&symbols[SIZE_MAX]) and a heap-buffer-overflow write in > > +# collect_symbols(). > > + > > +# To regenerate the test files: > > +# > > +# testfile-unstrip-symtab0.stripped: a valid ET_REL object with a four > > +# entry .symtab (sh_size 0x60, sh_entsize 0x18). Any small ET_REL file > > +# with an unstrippable symbol table will do. > > +# testfile-unstrip-symtab0.debug: the same file with the .symtab section > > +# header's sh_size field changed from 0x60 to 0x00. Offset 0x188, the > > +# low byte of that sh_size, is the only difference. > > +# testfile-unstrip-symtab1.stripped: a valid ET_REL object whose .symtab > > +# has sh_size 0x78 (120) and sh_entsize 0x79 (121). Both fields are > > +# needed: a nonzero sh_size keeps libelf willing to return the symbol > > +# data, while sh_entsize > sh_size drives the quotient to zero. > > Symbols > > +# 1..4 are STT_SECTION entries with st_shndx equal to their index. > > +# testfile-unstrip-symtab1.debug: the symtab0 debug file with its .symtab > > +# section header retyped SHT_PROGBITS (so it is not seen as a symbol > > +# table) plus one extra unnamed SHT_PROGBITS section header appended. > > + > > +. $srcdir/test-subr.sh > > + > > +stripped0=testfile-unstrip-symtab0.stripped > > +debug0=testfile-unstrip-symtab0.debug > > +stripped1=testfile-unstrip-symtab1.stripped > > +debug1=testfile-unstrip-symtab1.debug > > + > > +testfiles $stripped0 $debug0 $stripped1 $debug1 > > +tempfiles symtab-count.out symtab-count.err > > + > > +# Case 0: the unstripped (debug) file has an empty symbol table. This used > > +# to corrupt the heap in collect_symbols(). > > +if testrun ${abs_top_builddir}/src/unstrip -o symtab-count.out $stripped0 > > $debug0 \ > > + 2> symtab-count.err > > +then > > + echo >&2 "unstrip accepted an unstripped SYMTAB with sh_size smaller > > than sh_entsize" > > + exit 1 > > +fi > > + > > +grep -q ": unstripped SYMTAB section cannot" symtab-count.err > > + > > +# Case 1: the stripped file has sh_entsize larger than sh_size, and its > > +# debug file has no symbol table. The section copy loop rejects the > > +# malformed symbol table. > > +if testrun ${abs_top_builddir}/src/unstrip -o symtab-count.out $stripped1 > > $debug1 \ > > + 2> symtab-count.err > > +then > > + echo >&2 "unstrip accepted a stripped SYMTAB with sh_size smaller than > > sh_entsize" > > + exit 1 > > +fi > > + > > +grep -q ": SYMTAB section cannot" symtab-count.err > > + > > +# Case 2: symtab0 files with arguments swapped, so the malformed file is > > +# the stripped one and the debug file is valid. This used to corrupt the > > +# heap in collect_symbols() by passing an underflowed pointer > > +# (&symbols[SIZE_MAX]) when merging symbols. > > +if testrun ${abs_top_builddir}/src/unstrip -o symtab-count.out $debug0 > > $stripped0 \ > > + 2> symtab-count.err > > +then > > + echo >&2 "unstrip accepted a stripped SYMTAB with sh_size smaller than > > sh_entsize" > > + exit 1 > > +fi > > + > > +grep -q ": stripped SYMTAB section cannot" symtab-count.err > > + > > +test_cleanup > > + > > +exit 0 > > diff --git a/tests/testfile-unstrip-symtab0.debug.bz2 > > b/tests/testfile-unstrip-symtab0.debug.bz2 > > new file mode 100644 > > index > > 0000000000000000000000000000000000000000..b6ec646cdef78c62925f53df5693498c1426bfa3 > > GIT binary patch > > literal 220 > > zcmZ>Y%CIzaj8qGbeAA*Ez`)S;|MmYrKOQhJGB`WCFgfg2mP=^RaA06?P+(wkP+-`= > > zu%<a!<>Jc8j$3)BNHIt;<a8u3NHH+0H83zcsG3Y-Yj$gpVqjp%(6}b?S~%t7BHp|= > > z=N5TyydZQgQh2Ig2cxE<i<pDLOJgNA-jmDD{R}oVGF+ymz-evcrJ7|op)E^b8voLa > > z(51yudp!@#HF=>hYf_Jcih%P|pHGU5RQInfyDl5g>{uy#_iNl#hSZ<$`~ECl^U&nw > > bBmt9P#vrwXEq(7Fz4KVkB_uncbrk~uIQCP^ > > > > literal 0 > > HcmV?d00001 > > > > diff --git a/tests/testfile-unstrip-symtab0.stripped.bz2 > > b/tests/testfile-unstrip-symtab0.stripped.bz2 > > new file mode 100644 > > index > > 0000000000000000000000000000000000000000..17dfc2de9b62bf4e8e90ce936d2d4f524b62d61e > > GIT binary patch > > literal 225 > > zcmZ>Y%CIzaj8qGb)IIy#gn^;=|LgyMemr1cWN>zNVRHDbESJ!r;lRM)puoW7pun(! > > zVNDCCzvru5U*;^i<ts86Oc-tl@-Q$kFkHxBb}(TUQpsF!aRJ*EkU->{9ek}`t8>3` > > zIY}nkOiP+0@-E`)#DXnMLKD_BwkWZElN4m(J#pAN)kQ+FS1?d0_04(zuq7{^r*&So > > z+_IKy%Nf(v7dI%iWH@;AWGQrduq#|}UFxuBrreh6K2vJg_VAx5Kl`<AT0`nD!%3Om > > efs4~Zw>XP02+%DEb5H+Rt`gkJ$#m!bW(NSW=2=$& > > > > literal 0 > > HcmV?d00001 > > > > diff --git a/tests/testfile-unstrip-symtab1.debug.bz2 > > b/tests/testfile-unstrip-symtab1.debug.bz2 > > new file mode 100644 > > index > > 0000000000000000000000000000000000000000..36bb79c3bddb1d7eacac20842b5b96808db853da > > GIT binary patch > > literal 224 > > zcmZ>Y%CIzaj8qGbv@K$CU|^W=|MmYrKOQhNGB`WCFgfg2mP=^RaA06?P+(wkP+-`= > > zu%_9`XK|*9bl@@t23`hZ7Xt<c24=?vybKq3JQgsVFyZMBxWK^dm|0@gH8mth{jAx` > > z^oiRJsjxkM@}?{11A|m5W2aA}N>7@@1asw?jy{coN=Ay2Oswb5eEBL`*H>2M*Lz0! > > z%7uV^mse_SU~0*5u$1bmR&3xD_#*A3VJM-&C-v)cs!sVq3w!0g^5HM@IqFxhoj$AT > > c;zzgD{Z(S&y{^KoZPWgX7r9ylOaS=^06jTVKmY&$ > > > > literal 0 > > HcmV?d00001 > > > > diff --git a/tests/testfile-unstrip-symtab1.stripped.bz2 > > b/tests/testfile-unstrip-symtab1.stripped.bz2 > > new file mode 100644 > > index > > 0000000000000000000000000000000000000000..69c678a961c04ca96d6518d5592b48d9f1c1c9b2 > > GIT binary patch > > literal 179 > > zcmZ>Y%CIzaj8qGbe0!t+C<8;q|JU^=UMyl@WN>zNVRA5Hl1XULaA06?P+(wkP+;if > > z4L)hYxLEV$LWy2U-U)`HX3LkHVD@Fc!Y0@;`RCU6^;5kpi)TIaIJ!YWa`F9?t$KY5 > > z%8Hr{>_rTXw|Im&8AMAhSTgqBEA@KzF<mJ5((yyrHlK1mA+aWKp@2e1O2p#5;(>=w > > k7i|+<5hvR9u#DeDoXN}~R%nW)UhAESej+N?Wy&8N05sP`Z2$lO > > > > literal 0 > > HcmV?d00001 > > > > -- > > 2.55.0 > > > > >
