From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1062) id 168AE3858288; Fri, 11 Nov 2022 07:36:21 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 168AE3858288 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Alan Modra To: bfd-cvs@sourceware.org Subject: [binutils-gdb] PR28834, PR26946 sanity checking section size X-Act-Checkin: binutils-gdb X-Git-Author: Alan Modra X-Git-Refname: refs/heads/master X-Git-Oldrev: d0e5049d8fa8e1994a42bb0296f72cc981b8ce8c X-Git-Newrev: f7502dfe3f99d09fba2fc49f806ccc6b0a18c06d Message-Id: <20221111073621.168AE3858288@sourceware.org> Date: Fri, 11 Nov 2022 07:36:21 +0000 (GMT) X-BeenThere: binutils-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Nov 2022 07:36:21 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Df7502dfe3f99= d09fba2fc49f806ccc6b0a18c06d commit f7502dfe3f99d09fba2fc49f806ccc6b0a18c06d Author: Alan Modra Date: Fri Nov 11 13:43:42 2022 +1030 PR28834, PR26946 sanity checking section size =20 This patch provides a new function to sanity check section sizes. It's mostly extracted from what we had in bfd_get_full_section_contents but also handles compressed debug sections. Improvements are: - section file offset is taken into account, - added checks that a compressed section can be read from file. =20 The function is then used when handling multiple .debug_* sections that need to be read into a single buffer, to sanity check sizes before allocating the buffer. =20 PR 26946, PR 28834 * Makefile.am (LIBBFD_H_FILES): Add section.c. * compress.c (bfd_get_full_section_contents): Move section size sanity checks.. * section.c (_bfd_section_size_insane): ..to here. New functio= n. * dwarf2.c (read_section): Use _bfd_section_size_insane. (_bfd_dwarf2_slurp_debug_info): Likewise. * Makefile.in: Regenerate. * libbfd.h: Regenerate. Diff: --- bfd/Makefile.am | 2 +- bfd/Makefile.in | 2 +- bfd/compress.c | 43 ++++++++++++------------------------- bfd/dwarf2.c | 20 ++++++----------- bfd/libbfd.h | 6 +++++- bfd/section.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++= ++++ 6 files changed, 94 insertions(+), 45 deletions(-) diff --git a/bfd/Makefile.am b/bfd/Makefile.am index b70d8f3ccea..93313778d42 100644 --- a/bfd/Makefile.am +++ b/bfd/Makefile.am @@ -929,7 +929,7 @@ BFD_H_FILES =3D bfd-in.h init.c opncls.c libbfd.c \ linker.c simple.c compress.c BFD64_H_FILES =3D archive64.c LIBBFD_H_FILES =3D libbfd-in.h libbfd.c bfdio.c bfdwin.c \ - cache.c reloc.c targets.c archures.c linker.c + cache.c reloc.c section.c targets.c archures.c linker.c LIBCOFF_H_FILES =3D libcoff-in.h coffcode.h =20 headers: stmp-bin2-h stmp-lbfd-h stmp-lcoff-h diff --git a/bfd/Makefile.in b/bfd/Makefile.in index b906976a1c0..85cae1f6f0f 100644 --- a/bfd/Makefile.in +++ b/bfd/Makefile.in @@ -1223,7 +1223,7 @@ BFD_H_FILES =3D bfd-in.h init.c opncls.c libbfd.c \ =20 BFD64_H_FILES =3D archive64.c LIBBFD_H_FILES =3D libbfd-in.h libbfd.c bfdio.c bfdwin.c \ - cache.c reloc.c targets.c archures.c linker.c + cache.c reloc.c section.c targets.c archures.c linker.c =20 LIBCOFF_H_FILES =3D libcoff-in.h coffcode.h =20 diff --git a/bfd/compress.c b/bfd/compress.c index 9608a0a6341..ad3feeafc6c 100644 --- a/bfd/compress.c +++ b/bfd/compress.c @@ -244,7 +244,7 @@ DESCRIPTION bool bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr) { - bfd_size_type sz; + bfd_size_type sz =3D bfd_get_section_limit_octets (abfd, sec); bfd_byte *p =3D *ptr; bool ret; bfd_size_type save_size; @@ -253,45 +253,30 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec= , bfd_byte **ptr) unsigned int compression_header_size; const unsigned int compress_status =3D sec->compress_status; =20 - if (abfd->direction !=3D write_direction && sec->rawsize !=3D 0) - sz =3D sec->rawsize; - else - sz =3D sec->size; if (sz =3D=3D 0) { *ptr =3D NULL; return true; } =20 + if (p =3D=3D NULL + && compress_status !=3D COMPRESS_SECTION_DONE + && _bfd_section_size_insane (abfd, sec)) + { + /* PR 24708: Avoid attempts to allocate a ridiculous amount + of memory. */ + _bfd_error_handler + /* xgettext:c-format */ + (_("error: %pB(%pA) is too large (%#" PRIx64 " bytes)"), + abfd, sec, (uint64_t) sz); + return false; + } + switch (compress_status) { case COMPRESS_SECTION_NONE: if (p =3D=3D NULL) { - ufile_ptr filesize =3D bfd_get_file_size (abfd); - if (filesize > 0 - && filesize < sz - && (bfd_section_flags (sec) & SEC_IN_MEMORY) =3D=3D 0 - /* PR 24753: Linker created sections can be larger than - the file size, eg if they are being used to hold stubs. */ - && (bfd_section_flags (sec) & SEC_LINKER_CREATED) =3D=3D 0 - /* PR 24753: Sections which have no content should also be - excluded as they contain no size on disk. */ - && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) !=3D 0 - /* The MMO file format supports its own special compression - technique, but it uses COMPRESS_SECTION_NONE when loading - a section's contents. */ - && bfd_get_flavour (abfd) !=3D bfd_target_mmo_flavour) - { - /* PR 24708: Avoid attempts to allocate a ridiculous amount - of memory. */ - bfd_set_error (bfd_error_file_truncated); - _bfd_error_handler - /* xgettext:c-format */ - (_("error: %pB(%pA) section size (%#" PRIx64 " bytes) is larger than fil= e size (%#" PRIx64 " bytes)"), - abfd, sec, (uint64_t) sz, (uint64_t) filesize); - return false; - } p =3D (bfd_byte *) bfd_malloc (sz); if (p =3D=3D NULL) { diff --git a/bfd/dwarf2.c b/bfd/dwarf2.c index 364cc9a6480..95f45708e9d 100644 --- a/bfd/dwarf2.c +++ b/bfd/dwarf2.c @@ -690,7 +690,6 @@ read_section (bfd *abfd, { bfd_size_type amt; asection *msec; - ufile_ptr filesize; =20 msec =3D bfd_get_section_by_name (abfd, section_name); if (msec =3D=3D NULL) @@ -706,20 +705,14 @@ read_section (bfd *abfd, return false; } =20 - amt =3D bfd_get_section_limit_octets (abfd, msec); - filesize =3D bfd_get_file_size (abfd); - /* PR 28834: A compressed debug section could well decompress to a s= ize - larger than the file, so we choose an arbitrary modifier of 10x in - the test below. If this ever turns out to be insufficient, it can - be changed by a future update. */ - if (amt >=3D filesize * 10) + if (_bfd_section_size_insane (abfd, msec)) { /* PR 26946 */ - _bfd_error_handler (_("DWARF error: section %s is larger than 10x its f= ilesize! (0x%lx vs 0x%lx)"), - section_name, (long) amt, (long) filesize); - bfd_set_error (bfd_error_bad_value); + _bfd_error_handler (_("DWARF error: section %s is too big"), + section_name); return false; } + amt =3D bfd_get_section_limit_octets (abfd, msec); *section_size =3D amt; /* Paranoia - alloc one extra so that we can make sure a string section is NUL terminated. */ @@ -5496,9 +5489,10 @@ _bfd_dwarf2_slurp_debug_info (bfd *abfd, bfd *debug_= bfd, msec; msec =3D find_debug_info (debug_bfd, debug_sections, msec)) { + if (_bfd_section_size_insane (debug_bfd, msec)) + return false; /* Catch PR25070 testcase overflowing size calculation here. */ - if (total_size + msec->size < total_size - || total_size + msec->size < msec->size) + if (total_size + msec->size < total_size) { bfd_set_error (bfd_error_no_memory); return false; diff --git a/bfd/libbfd.h b/bfd/libbfd.h index a67bded0d18..2dec8109bb8 100644 --- a/bfd/libbfd.h +++ b/bfd/libbfd.h @@ -1,6 +1,7 @@ /* DO NOT EDIT! -*- buffer-read-only: t -*- This file is automatically generated from "libbfd-in.h", "libbfd.c", "bfdio.c", "bfdwin.c", - "cache.c", "reloc.c", "targets.c", "archures.c" and "linker.c". + "cache.c", "reloc.c", "section.c", "targets.c", "archures.c" + and "linker.c". Run "make headers" in your build bfd/ to regenerate. */ =20 /* libbfd.h -- Declarations used by bfd library *implementation*. @@ -3549,6 +3550,9 @@ bool _bfd_unrecognized_reloc sec_ptr section, unsigned int r_type); =20 +/* Extracted from section.c. */ +bool _bfd_section_size_insane (bfd *abfd, asection *sec); + /* Extracted from targets.c. */ const char **_bfd_per_xvec_warn (const bfd_target *); =20 diff --git a/bfd/section.c b/bfd/section.c index 614570e976e..48505f373e2 100644 --- a/bfd/section.c +++ b/bfd/section.c @@ -1703,3 +1703,69 @@ _bfd_nowrite_set_section_contents (bfd *abfd, { return _bfd_bool_bfd_false_error (abfd); } + +/* +INTERNAL_FUNCTION + _bfd_section_size_insane + +SYNOPSIS + bool _bfd_section_size_insane (bfd *abfd, asection *sec); + +DESCRIPTION + Returns true if the given section has a size that indicates + it cannot be read from file. Return false if the size is OK + *or* this function can't say one way or the other. + +*/ + +bool +_bfd_section_size_insane (bfd *abfd, asection *sec) +{ + bfd_size_type size =3D bfd_get_section_limit_octets (abfd, sec); + if (size =3D=3D 0) + return false; + + if ((bfd_section_flags (sec) & SEC_IN_MEMORY) !=3D 0 + /* PR 24753: Linker created sections can be larger than + the file size, eg if they are being used to hold stubs. */ + || (bfd_section_flags (sec) & SEC_LINKER_CREATED) !=3D 0 + /* PR 24753: Sections which have no content should also be + excluded as they contain no size on disk. */ + || (bfd_section_flags (sec) & SEC_HAS_CONTENTS) =3D=3D 0 + /* The MMO file format supports its own special compression + technique, but it uses COMPRESS_SECTION_NONE when loading + a section's contents. */ + || bfd_get_flavour (abfd) =3D=3D bfd_target_mmo_flavour) + return false; + + ufile_ptr filesize =3D bfd_get_file_size (abfd); + if (filesize =3D=3D 0) + return false; + + if (sec->compress_status =3D=3D DECOMPRESS_SECTION_ZSTD + || sec->compress_status =3D=3D DECOMPRESS_SECTION_ZLIB) + { + /* PR26946, PR28834: Sanity check compress header uncompressed + size against the original file size, and check that the + compressed section can be read from file. We choose an + arbitrary uncompressed size of 10x the file size, rather than + a compress ratio. The reason being that compiling + "int aaa..a;" with "a" repeated enough times can result in + compression ratios without limit for .debug_str, whereas such + a file will usually also have the enormous symbol + uncompressed in .symtab. */ + if (size / 10 > filesize) + { + bfd_set_error (bfd_error_bad_value); + return true; + } + size =3D sec->compressed_size; + } + + if ((ufile_ptr) sec->filepos > filesize || size > filesize - sec->filepo= s) + { + bfd_set_error (bfd_error_file_truncated); + return true; + } + return false; +}