Hi Aleksei,
On Tue, Feb 14, 2023 at 08:30:02PM +0000, Aleksei Vetrov via Elfutils-devel
wrote:
> It is expected from libdw to return strings that are null-terminated to
> avoid overflowing ELF data.
>
> * Add calculation of a safe prefix inside string sections, where any
> string will be null-terminated.
>
> * Check if offset overflows the safe prefix in dwarf_formstring.
This is a very nice sanity/hardening fix.
> + /* If the section contains string data, we want to know a size of a prefix
> + where any string will be null-terminated. */
> + enum string_section_index string_section_idx =
> scn_to_string_section_idx[cnt];
> + if (string_section_idx < STR_SCN_IDX_last)
> + {
> + size_t size = data->d_size;
> + /* Reduce the size by the number of non-zero bytes at the end of the
> + section. */
> + while (size > 0 && *((const char *) data->d_buf + size - 1) != '\0')
> + --size;
> + result->string_section_size[string_section_idx] = size;
> + }
Checking from the end is smart. In the normal case the debug string
section will end with a zero terminator (or zero padding), so this
should be really quick.
> @@ -171,7 +174,7 @@ dwarf_formstring (Dwarf_Attribute *attrp)
> else
> off = read_8ubyte_unaligned (dbg, datap);
>
> - if (off > dbg->sectiondata[IDX_debug_str]->d_size)
> + if (off >= data_size)
> goto invalid_offset;
> }
O, the original check was actually one off.
Looks good. Pushed as is.
Thanks,
Mark
