From f7502dfe3f99d09fba2fc49f806ccc6b0a18c06d Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Fri, 11 Nov 2022 13:43:42 +1030 Subject: [PATCH] PR28834, PR26946 sanity checking section size 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. 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. 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 function. * dwarf2.c (read_section): Use _bfd_section_size_insane. (_bfd_dwarf2_slurp_debug_info): Likewise. * Makefile.in: Regenerate. * libbfd.h: Regenerate. --- 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 = bfd-in.h init.c opncls.c libbfd.c \ linker.c simple.c compress.c BFD64_H_FILES = archive64.c LIBBFD_H_FILES = 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 = libcoff-in.h coffcode.h 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 = bfd-in.h init.c opncls.c libbfd.c \ BFD64_H_FILES = archive64.c LIBBFD_H_FILES = 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 = libcoff-in.h coffcode.h 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 = bfd_get_section_limit_octets (abfd, sec); bfd_byte *p = *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 = sec->compress_status; - if (abfd->direction != write_direction && sec->rawsize != 0) - sz = sec->rawsize; - else - sz = sec->size; if (sz == 0) { *ptr = NULL; return true; } + if (p == NULL + && compress_status != 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 == NULL) { - ufile_ptr filesize = bfd_get_file_size (abfd); - if (filesize > 0 - && filesize < sz - && (bfd_section_flags (sec) & SEC_IN_MEMORY) == 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) == 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) != 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) != 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 file size (%#" PRIx64 " bytes)"), - abfd, sec, (uint64_t) sz, (uint64_t) filesize); - return false; - } p = (bfd_byte *) bfd_malloc (sz); if (p == 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; msec = bfd_get_section_by_name (abfd, section_name); if (msec == NULL) @@ -706,20 +705,14 @@ read_section (bfd *abfd, return false; } - amt = bfd_get_section_limit_octets (abfd, msec); - filesize = bfd_get_file_size (abfd); - /* PR 28834: A compressed debug section could well decompress to a size - 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 >= filesize * 10) + if (_bfd_section_size_insane (abfd, msec)) { /* PR 26946 */ - _bfd_error_handler (_("DWARF error: section %s is larger than 10x its filesize! (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 = bfd_get_section_limit_octets (abfd, msec); *section_size = 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 = 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. */ /* libbfd.h -- Declarations used by bfd library *implementation*. @@ -3549,6 +3550,9 @@ bool _bfd_unrecognized_reloc sec_ptr section, unsigned int r_type); +/* 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 *); 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 = bfd_get_section_limit_octets (abfd, sec); + if (size == 0) + return false; + + if ((bfd_section_flags (sec) & SEC_IN_MEMORY) != 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) != 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) == 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) == bfd_target_mmo_flavour) + return false; + + ufile_ptr filesize = bfd_get_file_size (abfd); + if (filesize == 0) + return false; + + if (sec->compress_status == DECOMPRESS_SECTION_ZSTD + || sec->compress_status == 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 = sec->compressed_size; + } + + if ((ufile_ptr) sec->filepos > filesize || size > filesize - sec->filepos) + { + bfd_set_error (bfd_error_file_truncated); + return true; + } + return false; +} -- 2.30.2