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]> --- 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
