Re: RFC: fix bug in pieced value with offset

[Jan Kratochvil <jan.kratochvil@redhat.com>]

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