libdw: Add overflow checking to __libdw_form_val_len.
Pass endp as argument to __libdw_form_val_len and check we don't read
beyond the end of expected data and don't return lengths that would
overflow.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 909fdbc..b9b868b 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,13 @@
+2014-12-04 Mark Wielaard <mjw@redhat.com>
+
+ * libdwP.h (__libdw_form_val_compute_len): Add endp argument.
+ (__libdw_form_val_len): Likewise and check len doesn't overflow.
+ * libdw_form.c (__libdw_form_val_compute_len): Likewise.
+ * dwarf_child.c (__libdw_find_attr): Call __libdw_form_val_len
+ with endp.
+ * dwarf_getattrs.c (dwarf_getattrs): Likewise.
+ * dwarf_getmacros.c (read_macros): Likewise and check for errors.
+
2014-12-02 Petr Machata <pmachata@redhat.com>
* dwarf_getmacros.c (token_from_offset, offset_from_token): New
diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c
index 1d3a337..fa31600 100644
--- a/libdw/dwarf_child.c
+++ b/libdw/dwarf_child.c
@@ -1,5 +1,5 @@
/* Return child of current DIE.
- Copyright (C) 2003-2011 Red Hat, Inc.
+ Copyright (C) 2003-2011, 2014 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper <drepper@redhat.com>, 2003.
@@ -104,7 +104,8 @@
/* Skip over the rest of this attribute (if there is any). */
if (attr_form != 0)
{
- size_t len = __libdw_form_val_len (dbg, die->cu, attr_form, readp);
+ size_t len = __libdw_form_val_len (dbg, die->cu, attr_form, readp,
+ endp);
if (unlikely (len == (size_t) -1l))
{
@@ -112,7 +113,7 @@
break;
}
- // XXX We need better boundary checks.
+ // __libdw_form_val_len will have done a bounds check.
readp += len;
}
}
diff --git a/libdw/dwarf_getattrs.c b/libdw/dwarf_getattrs.c
index 82eb3f8..627a851 100644
--- a/libdw/dwarf_getattrs.c
+++ b/libdw/dwarf_getattrs.c
@@ -1,5 +1,5 @@
/* Get attributes of the DIE.
- Copyright (C) 2004, 2005, 2008, 2009 Red Hat, Inc.
+ Copyright (C) 2004, 2005, 2008, 2009, 2014 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper <drepper@redhat.com>, 2004.
@@ -67,12 +67,13 @@
/* Go over the list of attributes. */
Dwarf *dbg = die->cu->dbg;
+ const unsigned char *endp;
+ endp = ((const unsigned char *) dbg->sectiondata[IDX_debug_abbrev]->d_buf
+ + dbg->sectiondata[IDX_debug_abbrev]->d_size);
while (1)
{
/* Are we still in bounds? */
- if (unlikely (attrp
- >= ((unsigned char *) dbg->sectiondata[IDX_debug_abbrev]->d_buf
- + dbg->sectiondata[IDX_debug_abbrev]->d_size)))
+ if (unlikely (attrp >= endp))
goto invalid_dwarf;
/* Get attribute name and form. */
@@ -111,13 +112,13 @@
if (attr.form != 0)
{
size_t len = __libdw_form_val_len (dbg, die->cu, attr.form,
- die_addr);
+ die_addr, endp);
if (unlikely (len == (size_t) -1l))
/* Something wrong with the file. */
return -1l;
- // XXX We need better boundary checks.
+ // __libdw_form_val_len will have done a bounds check.
die_addr += len;
}
}
diff --git a/libdw/dwarf_getmacros.c b/libdw/dwarf_getmacros.c
index bd64d60..737dc5d 100644
--- a/libdw/dwarf_getmacros.c
+++ b/libdw/dwarf_getmacros.c
@@ -367,8 +367,12 @@
attributes[i].valp = (void *) readp;
attributes[i].cu = &fake_cu;
- readp += __libdw_form_val_len (dbg, &fake_cu,
- proto->forms[i], readp);
+ size_t len = __libdw_form_val_len (dbg, &fake_cu,
+ proto->forms[i], readp, endp);
+ if (len == (size_t) -1)
+ return -1;
+
+ readp += len;
}
Dwarf_Macro macro = {
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index 5ccb13c..351c5b4 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -458,14 +458,16 @@
/* Helper functions for form handling. */
extern size_t __libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
unsigned int form,
- const unsigned char *valp)
- __nonnull_attribute__ (1, 2, 4) internal_function;
+ const unsigned char *valp,
+ const unsigned char *endp)
+ __nonnull_attribute__ (1, 2, 4, 5) internal_function;
/* Find the length of a form attribute. */
static inline size_t
-__nonnull_attribute__ (1, 2, 4)
+__nonnull_attribute__ (1, 2, 4, 5)
__libdw_form_val_len (Dwarf *dbg, struct Dwarf_CU *cu,
- unsigned int form, const unsigned char *valp)
+ unsigned int form, const unsigned char *valp,
+ const unsigned char *endp)
{
/* Small lookup table of forms with fixed lengths. Absent indexes are
initialized 0, so any truly desired 0 is set to 0x80 and masked. */
@@ -483,11 +485,19 @@
{
uint8_t len = form_lengths[form];
if (len != 0)
- return len & 0x7f; /* Mask to allow 0x80 -> 0. */
+ {
+ len &= 0x7f; /* Mask to allow 0x80 -> 0. */
+ if (unlikely (len > (size_t) (endp - valp)))
+ {
+ __libdw_seterrno (DWARF_E_INVALID_DWARF);
+ return -1;
+ }
+ return len;
+ }
}
/* Other forms require some computation. */
- return __libdw_form_val_compute_len (dbg, cu, form, valp);
+ return __libdw_form_val_compute_len (dbg, cu, form, valp, endp);
}
/* Helper function for DW_FORM_ref* handling. */
diff --git a/libdw/libdw_form.c b/libdw/libdw_form.c
index 5350556..65ac7de 100644
--- a/libdw/libdw_form.c
+++ b/libdw/libdw_form.c
@@ -1,5 +1,5 @@
/* Helper functions for form handling.
- Copyright (C) 2003-2009 Red Hat, Inc.
+ Copyright (C) 2003-2009, 2014 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper <drepper@redhat.com>, 2003.
@@ -40,9 +40,10 @@
size_t
internal_function
__libdw_form_val_compute_len (Dwarf *dbg, struct Dwarf_CU *cu,
- unsigned int form, const unsigned char *valp)
+ unsigned int form, const unsigned char *valp,
+ const unsigned char *endp)
{
- const unsigned char *saved;
+ const unsigned char *startp = valp;
Dwarf_Word u128;
size_t result;
@@ -66,49 +67,67 @@
break;
case DW_FORM_block1:
+ if (unlikely ((size_t) (endp - startp) < 1))
+ goto invalid;
result = *valp + 1;
break;
case DW_FORM_block2:
+ if (unlikely ((size_t) (endp - startp) < 2))
+ goto invalid;
result = read_2ubyte_unaligned (dbg, valp) + 2;
break;
case DW_FORM_block4:
+ if (unlikely ((size_t) (endp - startp) < 4))
+ goto invalid;
result = read_4ubyte_unaligned (dbg, valp) + 4;
break;
case DW_FORM_block:
case DW_FORM_exprloc:
- saved = valp;
+ // XXX overflow check
get_uleb128 (u128, valp);
- result = u128 + (valp - saved);
+ result = u128 + (valp - startp);
break;
case DW_FORM_string:
- result = strlen ((char *) valp) + 1;
- break;
+ {
+ const unsigned char *endstrp = memchr (valp, '\0',
+ (size_t) (endp - startp));
+ if (unlikely (endstrp == NULL))
+ goto invalid;
+ result = (size_t) (endstrp - startp) + 1;
+ break;
+ }
case DW_FORM_sdata:
case DW_FORM_udata:
case DW_FORM_ref_udata:
- saved = valp;
+ // XXX overflow check
get_uleb128 (u128, valp);
- result = valp - saved;
+ result = valp - startp;
break;
case DW_FORM_indirect:
- saved = valp;
get_uleb128 (u128, valp);
// XXX Is this really correct?
- result = __libdw_form_val_len (dbg, cu, u128, valp);
+ result = __libdw_form_val_len (dbg, cu, u128, valp, endp);
if (result != (size_t) -1)
- result += valp - saved;
+ result += valp - startp;
+ else
+ return (size_t) -1;
break;
default:
+ goto invalid;
+ }
+
+ if (unlikely (result > (size_t) (endp - startp)))
+ {
+ invalid:
__libdw_seterrno (DWARF_E_INVALID_DWARF);
- result = (size_t) -1l;
- break;
+ result = (size_t) -1;
}
return result;