[binutils-gdb] alpha_vms_get_section_contents vs. fuzzed files

[binutils-gdb] alpha_vms_get_section_contents vs. fuzzed files

[Alan Modra]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=ee19a4725c01f4924657a1d6f09f0e4dcd6bba17

commit ee19a4725c01f4924657a1d6f09f0e4dcd6bba17
Author: Alan Modra <amodra@gmail.com>
Date:   Wed Apr 17 18:29:19 2024 +0930

    alpha_vms_get_section_contents vs. fuzzed files
    
    This patch is in response to an oss-fuzz report regarding
    use-of-uninitialized-value in bfd_is_section_compressed_info from
    section contents provided by alpha_vms_get_section_contents.  That
    hole is covered by using bfd_zalloc rather than bfd_alloc.
    
    The rest of the patch is mostly a tidy.  In a function returning
    section contents, I tend to prefer a test on the section properties
    over a test on file properties.  That's why I've changed the file
    flags test to one on section filepos and flags before calling
    _bfd_generic_get_section_contents.  Also, fuzzed objects can easily
    have sections with file backing in relocatable objects, or sections
    without file backing in images.  Possible confusion is avoided by
    testing each section.
    
    Note that we are always going to run into out-of-memory with fuzzed
    alpha-vms object files due to sections with contents via ETIR records.
    eg. ETIR__C_STO_IMMR stores a number of bytes repeatedly, with a
    32-bit repeat count.  So section contents can be very large from a
    relatively small file.  I'm inclined to think that an out-of-memory
    error is fine for such files.
    
            * vms-alpha.c (alpha_vms_get_section_contents): Handle sections
            with non-zero filepos or without SEC_HAS_CONTENTS via
            _bfd_generic_get_section_contents.  Zero memory allocated for
            sections filled by ETIR records.

Diff:
---
 bfd/vms-alpha.c | 50 ++++++++++++++++++++++++++------------------------
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/bfd/vms-alpha.c b/bfd/vms-alpha.c
index 6b896d0f4ee..8b5e3c21ac6 100644
--- a/bfd/vms-alpha.c
+++ b/bfd/vms-alpha.c
@@ -9834,13 +9834,16 @@ alpha_vms_get_section_contents (bfd *abfd, asection *section,
 				void *buf, file_ptr offset,
 				bfd_size_type count)
 {
-  asection *sec;
-
-  /* Image are easy.  */
-  if (bfd_get_file_flags (abfd) & (EXEC_P | DYNAMIC))
+  /* Handle image sections.  */
+  if (section->filepos != 0
+      || (section->flags & SEC_HAS_CONTENTS) == 0)
     return _bfd_generic_get_section_contents (abfd, section,
 					      buf, offset, count);
 
+  /* A section with a zero filepos implies the section has no direct
+     file backing.  Its contents must be calculated by processing ETIR
+     records.  */
+
   /* Safety check.  */
   if (offset + count < count
       || offset + count > section->size)
@@ -9849,33 +9852,32 @@ alpha_vms_get_section_contents (bfd *abfd, asection *section,
       return false;
     }
 
-  /* If the section is already in memory, just copy it.  */
-  if (section->flags & SEC_IN_MEMORY)
-    {
-      BFD_ASSERT (section->contents != NULL);
-      memcpy (buf, section->contents + offset, count);
-      return true;
-    }
   if (section->size == 0)
     return true;
 
-  /* Alloc in memory and read ETIRs.  */
-  for (sec = abfd->sections; sec; sec = sec->next)
+  /* If we haven't yet read ETIR/EDBG/ETBT records, do so.  */
+  if ((section->flags & SEC_IN_MEMORY) == 0)
     {
-      BFD_ASSERT (sec->contents == NULL);
-
-      if (sec->size != 0 && (sec->flags & SEC_HAS_CONTENTS))
+      /* Alloc memory and read ETIRs.  */
+      for (asection *sec = abfd->sections; sec; sec = sec->next)
 	{
-	  sec->contents = bfd_alloc (abfd, sec->size);
-	  if (sec->contents == NULL)
-	    return false;
+	  if (sec->size != 0
+	      && sec->filepos == 0
+	      && (sec->flags & SEC_HAS_CONTENTS) != 0)
+	    {
+	      BFD_ASSERT (sec->contents == NULL);
+
+	      sec->contents = bfd_zalloc (abfd, sec->size);
+	      sec->flags |= SEC_IN_MEMORY;
+	      if (sec->contents == NULL)
+		return false;
+	    }
 	}
+      if (!alpha_vms_read_sections_content (abfd, NULL))
+	return false;
     }
-  if (!alpha_vms_read_sections_content (abfd, NULL))
-    return false;
-  for (sec = abfd->sections; sec; sec = sec->next)
-    if (sec->contents)
-      sec->flags |= SEC_IN_MEMORY;
+
+  BFD_ASSERT (section->contents != NULL);
   memcpy (buf, section->contents + offset, count);
   return true;
 }