Hello, Please review the attached fix for Sourceware Bugzilla #34705: https://sourceware.org/bugzilla/show_bug.cgi?id=34705
The patch rejects SYMTAB/DYNSYM sections when sh_size is smaller than sh_entsize before symbol counts are used, preventing zero-count underflow in unstrip's merge and section-symbol paths. It adds regression coverage for malformed stripped and unstripped inputs. Thanks, Kind regards Harshit Kumar
From 46c8a031c22bde9da8d2ce62e02523d3fe4008cf Mon Sep 17 00:00:00 2001 From: Harshit Kumar <[email protected]> Date: Sat, 3 Oct 2026 15:03:18 +0530 Subject: [PATCH] unstrip: reject symbol tables whose sh_size is below sh_entsize 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 at line 1782 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`. Fix the guards at all sites so the quotient can never be zero. Add a regression test covering malformed stripped and unstripped symbol tables. * src/unstrip.c (add_new_section_symbols): Reject symbol table when sh_size < sh_entsize. (copy_elided_sections): Likewise for stripped and unstripped symbol tables. * tests/run-unstrip-symtab-count.sh: New test script. * tests/testfile-unstrip-symtab0.debug.bz2: New test file. * tests/testfile-unstrip-symtab0.stripped.bz2: Likewise. * tests/testfile-unstrip-symtab1.debug.bz2: Likewise. * tests/testfile-unstrip-symtab1.stripped.bz2: Likewise. * tests/Makefile.am (TESTS): Add run-unstrip-symtab-count.sh. (EXTRA_DIST): Add new test script and bz2 files. Signed-off-by: Harshit Kumar <[email protected]> --- src/unstrip.c | 25 +++-- tests/Makefile.am | 10 +- tests/run-unstrip-symtab-count.sh | 105 ++++++++++++++++++++ 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, 129 insertions(+), 11 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..24db7c45 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,10 @@ 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 +1846,23 @@ 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..404192ed 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 \ @@ -377,7 +378,12 @@ EXTRA_DIST = run-arextract.sh run-arsymtest.sh run-ar.sh \ testfile-info-link.stripped.bz2 run-unstrip-test3.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-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..6054c5ac --- /dev/null +++ b/tests/run-unstrip-symtab-count.sh @@ -0,0 +1,105 @@ +#! /bin/sh +# Copyright (C) 2026 Red Hat, Inc. +# 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. That skips the merge step above and reaches +# add_new_section_symbols(), where "size_t symndx_map[nsym - 1]" was +# declared with nsym == 0. Before the fix unstrip accepted this file and +# produced output; now it is rejected. +# +# symtab2: the stripped file has an empty symbol table while the debug file is +# valid. 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 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 a SYMTAB with sh_size smaller than sh_entsize" + exit 1 +fi + +grep "sh_size smaller than sh_entsize" symtab-count.err + +# Case 1: the stripped file has sh_entsize one larger than sh_size, and its +# debug file has no symbol table. This used to build a stack array declared +# with nsym - 1 == SIZE_MAX elements. +if testrun ${abs_top_builddir}/src/unstrip -o symtab-count.out $stripped1 $debug1 \ + 2> symtab-count.err +then + echo >&2 "unstrip accepted a SYMTAB with sh_size smaller than sh_entsize" + exit 1 +fi + +grep "sh_size smaller than sh_entsize" symtab-count.err + +# Case 2: the stripped file has an empty symbol table while 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 "sh_size smaller than sh_entsize" 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
