From 1cfcf3004e1830f8fe9112cfcd15285508d2c2b7 Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Thu, 11 Feb 2021 16:56:42 +1030 Subject: [PATCH] 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. --- binutils/ChangeLog | 14 +++++++++ binutils/od-elf32_avr.c | 66 ++++++++++++++++++++++++++--------------- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/binutils/ChangeLog b/binutils/ChangeLog index 594fbffdd66..96283905074 100644 --- a/binutils/ChangeLog +++ b/binutils/ChangeLog @@ -1,3 +1,17 @@ +2021-02-11 Alan Modra + + 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 PR binutils/27391 diff --git a/binutils/od-elf32_avr.c b/binutils/od-elf32_avr.c index 11800e76290..4377a7c652a 100644 --- 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, - ¬e_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, -- 2.30.2