PR28834, PR26946 sanity checking section size

PR28834, PR26946 sanity checking section size

[Alan Modra]

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