From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by sourceware.org (Postfix) with ESMTPS id 6F6CF3858025 for ; Fri, 11 Nov 2022 07:34:58 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 6F6CF3858025 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pj1-x102f.google.com with SMTP id m14-20020a17090a3f8e00b00212dab39bcdso7101802pjc.0 for ; Thu, 10 Nov 2022 23:34:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-disposition:mime-version:message-id:subject:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=YOygpStUCzUPEyx+pwXwdZDwpfyS3Qo5je/eUIglL/k=; b=lPhFVcbfm68jpRH9yEOEwn1p7u0Z3nESEeiXVJlBmZASaLxjhj7CsPD7UNnW6vBOTi ithGiCvQGfu6nKtThk/7YUsH+rrv0VnRgj5jTtDQSgIWyY+uUmJKJVbQCAnNFMEHirDn nozh/wUF+dHDVnrH3VJH/9u5KDEJTAd/0MkFnMwukXsoNZxQog/9s+D6ImvImEB9R2OF TPmThuWJFW/Oun7nWsa4jo4ivlSL2LnDiUptDO9dwQEZIdzR7EsxXAxT2BTpRxW174+6 sg+ElijBPCcbepn+XHwABdumZGBFOnR+Cws84lgdmF+t/+wQiudrK1uiCXhwSh3dEpVc yxSg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-disposition:mime-version:message-id:subject:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=YOygpStUCzUPEyx+pwXwdZDwpfyS3Qo5je/eUIglL/k=; b=fidZsL2FX5gb60IrIzAoOgwa37Os5rH4laabSmlqaT3dSwstB2f17N1f8fIH3rT2WZ Cc8u9rR2/u6f2C2uWZehwlC9Deiqv2xKCYRmUSgsnCu2TJIy4Hho/SKtwIZw+O9Lo0K/ j9YPuZ19HbN0P3HTsVuGMLlfzRPAcRl/CWwliyPDx3HyaoJTVhDcPDEdNMnfLx9RWcgW 1PFZttDVF4wYp4J/Tt+nz+f4nQqpJlfDJA8fY8wRo0G5V8UPnKmRMynx7UKjxbvRHlmR YoUeAHmCkF+gu7BZsd5JFjIoyn8JF6qrYQ7gSPhomd88cXhNlWwjjB95IO9fy+UzPZXg i/bw== X-Gm-Message-State: ANoB5plnpgTeIIVw5rjerYEXzE81WlOTBmkqnRi2fFa/jgZjEOfUQFo9 KH0MmxtTgf0qwYD/wPMrsYe0nZCGfAE= X-Google-Smtp-Source: AA0mqf4RDvHycScyE/2nf6Qnh6FGaOfETyOcsPbegjOXJ+ku9EewvKPPLjTTMs6CZEevndcTS/ktKA== X-Received: by 2002:a17:902:b711:b0:186:a3a0:601c with SMTP id d17-20020a170902b71100b00186a3a0601cmr1394892pls.105.1668152096924; Thu, 10 Nov 2022 23:34:56 -0800 (PST) Received: from squeak.grove.modra.org ([2406:3400:51d:8cc0:2ea0:e07f:18a5:e51e]) by smtp.gmail.com with ESMTPSA id q18-20020aa79612000000b005609d3d3008sm902406pfg.171.2022.11.10.23.34.56 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 10 Nov 2022 23:34:56 -0800 (PST) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id D95E11142D8E; Fri, 11 Nov 2022 18:04:53 +1030 (ACDT) Date: Fri, 11 Nov 2022 18:04:53 +1030 From: Alan Modra To: binutils@sourceware.org Subject: PR28834, PR26946 sanity checking section size Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3035.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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. 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; +} -- Alan Modra Australia Development Lab, IBM