PR27290, PR27293, PR27295, various avr objdump fixes

Adds missing sanity checks for avr device info note, to avoid
potential buffer overflows.  Uses bfd_malloc_and_get_section for
sanity checking section size.

	PR 27290
	PR 27293
	PR 27295
	* od-elf32_avr.c (elf32_avr_get_note_section_contents): Formatting.
	Use bfd_malloc_and_get_section.
	(elf32_avr_get_note_desc): Formatting.  Return descsz.  Sanity
	check namesz.  Return NULL if descsz is too small.  Ensure
	string table is terminated.
	(elf32_avr_get_device_info): Formatting.  Add note_size param.
	Sanity check note.
	(elf32_avr_dump_mem_usage): Adjust to suit.

diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index 594fbffdd669294c832d6fb00997a9afcc3dd5c8..9628390507495555b30e66d832506ecb7eed3ad2 100644 (file)
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,17 @@
+2021-02-11  Alan Modra  <amodra@gmail.com>
+
+       PR 27290
+       PR 27293
+       PR 27295
+       * od-elf32_avr.c (elf32_avr_get_note_section_contents): Formatting.
+       Use bfd_malloc_and_get_section.
+       (elf32_avr_get_note_desc): Formatting.  Return descsz.  Sanity
+       check namesz.  Return NULL if descsz is too small.  Ensure
+       string table is terminated.
+       (elf32_avr_get_device_info): Formatting.  Add note_size param.
+       Sanity check note.
+       (elf32_avr_dump_mem_usage): Adjust to suit.
+
 2021-02-10  Tom de Vries  <tdevries@suse.de>
 
        PR binutils/27391

diff --git a/binutils/od-elf32_avr.c b/binutils/od-elf32_avr.c
index 11800e7629031d672d2ae6a33b049b1f4356b48f..4377a7c652a0704470acb9e4485dd9bf2e1e78b0 100644 (file)
--- a/binutils/od-elf32_avr.c
+++ b/binutils/od-elf32_avr.c
@@ -77,23 +77,29 @@ elf32_avr_filter (bfd *abfd)
   return bfd_get_flavour (abfd) == bfd_target_elf_flavour;
 }
 
-static char*
+static char *
 elf32_avr_get_note_section_contents (bfd *abfd, bfd_size_type *size)
 {
   asection *section;
+  bfd_byte *contents;
 
-  if ((section = bfd_get_section_by_name (abfd, ".note.gnu.avr.deviceinfo")) == NULL)
+  section = bfd_get_section_by_name (abfd, ".note.gnu.avr.deviceinfo");
+  if (section == NULL)
     return NULL;
 
-  *size = bfd_section_size (section);
-  char *contents = (char *) xmalloc (*size);
-  bfd_get_section_contents (abfd, section, contents, 0, *size);
+  if (!bfd_malloc_and_get_section (abfd, section, &contents))
+    {
+      free (contents);
+      contents = NULL;
+    }
 
-  return contents;
+  *size = bfd_section_size (section);
+  return (char *) contents;
 }
 
-static char* elf32_avr_get_note_desc (bfd *abfd, char *contents,
-        bfd_size_type size)
+static char *
+elf32_avr_get_note_desc (bfd *abfd, char *contents, bfd_size_type size,
+                        bfd_size_type *descsz)
 {
   Elf_External_Note *xnp = (Elf_External_Note *) contents;
   Elf_Internal_Note in;
@@ -107,42 +113,54 @@ static char* elf32_avr_get_note_desc (bfd *abfd, char *contents,
   if (in.namesz > contents - in.namedata + size)
     return NULL;
 
+  if (in.namesz != 4 || strcmp (in.namedata, "AVR") != 0)
+    return NULL;
+
   in.descsz = bfd_get_32 (abfd, xnp->descsz);
   in.descdata = in.namedata + align_power (in.namesz, 2);
-  if (in.descsz != 0
-        && (in.descdata >= contents + size
-            || in.descsz > contents - in.descdata + size))
+  if (in.descsz < 6 * sizeof (uint32_t)
+      || in.descdata >= contents + size
+      || in.descsz > contents - in.descdata + size)
     return NULL;
 
-  if (strcmp (in.namedata, "AVR") != 0)
-    return NULL;
+  /* If the note has a string table, ensure it is 0 terminated.  */
+  if (in.descsz > 8 * sizeof (uint32_t))
+    in.descdata[in.descsz - 1] = 0;
 
+  *descsz = in.descsz;
   return in.descdata;
 }
 
 static void
 elf32_avr_get_device_info (bfd *abfd, char *description,
-        deviceinfo *device)
+                          bfd_size_type desc_size, deviceinfo *device)
 {
   if (description == NULL)
     return;
 
   const bfd_size_type memory_sizes = 6;
 
-  memcpy (device, description, memory_sizes * sizeof(uint32_t));
-  device->name = NULL;
+  memcpy (device, description, memory_sizes * sizeof (uint32_t));
+  desc_size -= memory_sizes * sizeof (uint32_t);
+  if (desc_size < 8)
+    return;
 
-  uint32_t *stroffset_table = ((uint32_t *) description) + memory_sizes;
+  uint32_t *stroffset_table = (uint32_t *) description + memory_sizes;
   bfd_size_type stroffset_table_size = bfd_get_32 (abfd, stroffset_table);
-  char *str_table = ((char *) stroffset_table) + stroffset_table_size;
 
   /* If the only content is the size itself, there's nothing in the table */
-  if (stroffset_table_size == 4)
+  if (stroffset_table_size < 8)
     return;
+  if (desc_size <= stroffset_table_size)
+    return;
+  desc_size -= stroffset_table_size;
 
   /* First entry is the device name index. */
   uint32_t device_name_index = bfd_get_32 (abfd, stroffset_table + 1);
+  if (device_name_index >= desc_size)
+    return;
 
+  char *str_table = (char *) stroffset_table + stroffset_table_size;
   device->name = str_table + device_name_index;
 }
 
@@ -183,7 +201,7 @@ static void
 elf32_avr_dump_mem_usage (bfd *abfd)
 {
   char *description = NULL;
-  bfd_size_type note_section_size = 0;
+  bfd_size_type sec_size, desc_size;
 
   deviceinfo device = { 0, 0, 0, 0, 0, 0, NULL };
   device.name = "Unknown";
@@ -192,13 +210,13 @@ elf32_avr_dump_mem_usage (bfd *abfd)
   bfd_size_type text_usage = 0;
   bfd_size_type eeprom_usage = 0;
 
-  char *contents = elf32_avr_get_note_section_contents (abfd,
-    &note_section_size);
+  char *contents = elf32_avr_get_note_section_contents (abfd, &sec_size);
 
   if (contents != NULL)
     {
-      description = elf32_avr_get_note_desc (abfd, contents, note_section_size);
-      elf32_avr_get_device_info (abfd, description, &device);
+      description = elf32_avr_get_note_desc (abfd, contents, sec_size,
+                                            &desc_size);
+      elf32_avr_get_device_info (abfd, description, desc_size, &device);
     }
 
   elf32_avr_get_memory_usage (abfd, &text_usage, &data_usage,