This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: RFC: fix bug in pieced value with offset
On Fri, 14 May 2010 22:31:43 +0200, Tom Tromey wrote:
> What do you think of this revision?
Except one severe problem I find it OK now myself.
> case DWARF_VALUE_STACK:
> {
> struct gdbarch *gdbarch = get_type_arch (value_type (v));
> - size_t n = p->size;
> - if (n > c->addr_size)
> - n = c->addr_size;
> - store_unsigned_integer (contents + offset, n,
> - gdbarch_byte_order (gdbarch),
> - p->v.expr.value);
> + size_t n = this_size;
> + if (n > c->addr_size - source_offset)
> + n = c->addr_size - source_offset;
As the strict sanity checks are not included this expression may be an
exploitable memory corruption by overflown N in the case of: DW_OP_piece size
larger than CU address size and value_offset (still in the scope of this
DW_OP_piece size) also larger than CU address size. Suggesting something
like:
# size_t n = this_size;
# if (n > c->addr_size - source_offset)
# n = c->addr_size >= source_offset ? c->addr_size - source_offset : 0;
> case DWARF_VALUE_LITERAL:
> {
> - size_t n = p->size;
> - if (n > p->v.literal.length)
> - n = p->v.literal.length;
> - memcpy (contents + offset, p->v.literal.data, n);
> + if (this_size > p->v.literal.length - source_offset)
> + this_size = p->v.literal.length - source_offset;
again some:
# if (this_size > p->v.literal.length - source_offset)
# this_size = p->v.literal.length >= source_offset
# ? p->v.literal.length - source_offset : 0;
(DWARF_VALUE_STACK now does not modify THIS_SIZE while DWARF_VALUE_LITERAL
modifies it - thus corrupting OFFSET - but both only in the cases of invalid
DWARF I was suggesting to error() anyway so it probably does not matter.)
> +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {}] != "" } {
> + return -1
> +}
> +
> +clean_restart ${testfile}.x
pitpick: Therefore it could use prepare_for_testing now.
Thanks,
Jan