diff mbox series

PR31692, objdump fails .debug_info size check

Message ID ZjNz7wGwHoOSfCJV@squeak.grove.modra.org
State New
Headers
Series PR31692, objdump fails .debug_info size check |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Patch is already merged
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Patch is already merged

Commit Message

Alan Modra May 2, 2024, 11:07 a.m. UTC 
  PR 31692
	* objdump.c (load_specific_debug_section): Replace bfd_get_size
	check with bfd_section_size_insane.  Call free_debug_section
	after printing error messages.  Set section->start NULL when
	freeing.

Comments

Alan Modra May 10, 2024, 1:11 p.m. UTC | #1 
The fuzzers found a hole.  bfd_section_size_insane doesn't check
!SEC_HAS_CONTENTS sections against file size for obvious reasons,
which allows fuzzed debug sections to be stupidly large.  Real debug
sections of course always have contents.

	PR 31692
	* objdump.c (load_specific_debug_section): Don't allow sections
	without contents.

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 3d70df470f2..7182abdab98 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -4307,41 +4307,45 @@ load_specific_debug_section (enum dwarf_section_display_enum debug,
       return false;
     }
 
-  section->start = contents = xmalloc (alloced);
-  /* Ensure any string section has a terminating NUL.  */
-  section->start[section->size] = 0;
-
-  if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
-      && debug_displays [debug].relocate)
-    {
-      ret = bfd_simple_get_relocated_section_contents (abfd,
-						       sec,
-						       section->start,
-						       syms) != NULL;
-      if (ret)
-	{
-	  long reloc_size = bfd_get_reloc_upper_bound (abfd, sec);
+  ret = false;
+  if ((sec->flags & SEC_HAS_CONTENTS) != 0)
+    {
+      section->start = contents = xmalloc (alloced);
+      /* Ensure any string section has a terminating NUL.  */
+      section->start[section->size] = 0;
 
-	  if (reloc_size > 0)
+      if ((abfd->flags & (EXEC_P | DYNAMIC)) == 0
+	  && debug_displays [debug].relocate)
+	{
+	  ret = bfd_simple_get_relocated_section_contents (abfd,
+							   sec,
+							   section->start,
+							   syms) != NULL;
+	  if (ret)
 	    {
-	      long reloc_count;
-	      arelent **relocs;
+	      long reloc_size = bfd_get_reloc_upper_bound (abfd, sec);
 
-	      relocs = (arelent **) xmalloc (reloc_size);
-
-	      reloc_count = bfd_canonicalize_reloc (abfd, sec, relocs, syms);
-	      if (reloc_count <= 0)
-		free (relocs);
-	      else
+	      if (reloc_size > 0)
 		{
-		  section->reloc_info = relocs;
-		  section->num_relocs = reloc_count;
+		  long reloc_count;
+		  arelent **relocs;
+
+		  relocs = (arelent **) xmalloc (reloc_size);
+
+		  reloc_count = bfd_canonicalize_reloc (abfd, sec, relocs, syms);
+		  if (reloc_count <= 0)
+		    free (relocs);
+		  else
+		    {
+		      section->reloc_info = relocs;
+		      section->num_relocs = reloc_count;
+		    }
 		}
 	    }
 	}
+      else
+	ret = bfd_get_full_section_contents (abfd, sec, &contents);
     }
-  else
-    ret = bfd_get_full_section_contents (abfd, sec, &contents);
 
   if (!ret)
     {
diff mbox series

Patch

diff --git a/binutils/objdump.c b/binutils/objdump.c
index 5acaa54929d..3d70df470f2 100644
--- a/binutils/objdump.c
+++ b/binutils/objdump.c
@@ -4286,6 +4286,7 @@  load_specific_debug_section (enum dwarf_section_display_enum debug,
       if (streq (section->filename, bfd_get_filename (abfd)))
 	return true;
       free (section->start);
+      section->start = NULL;
     }
 
   section->filename = bfd_get_filename (abfd);
@@ -4297,13 +4298,12 @@  load_specific_debug_section (enum dwarf_section_display_enum debug,
   alloced = amt = section->size + 1;
   if (alloced != amt
       || alloced == 0
-      || (bfd_get_size (abfd) != 0 && alloced >= bfd_get_size (abfd)))
+      || bfd_section_size_insane (abfd, sec))
     {
-      section->start = NULL;
-      free_debug_section (debug);
       printf (_("\nSection '%s' has an invalid size: %#" PRIx64 ".\n"),
 	      sanitize_string (section->name),
 	      section->size);
+      free_debug_section (debug);
       return false;
     }
 
@@ -4345,9 +4345,9 @@  load_specific_debug_section (enum dwarf_section_display_enum debug,
 
   if (!ret)
     {
-      free_debug_section (debug);
       printf (_("\nCan't get contents for section '%s'.\n"),
 	      sanitize_string (section->name));
+      free_debug_section (debug);
       return false;
     }