Re: [PATCH] libdw: Return an error in dwarf_getlocation_attr for missing .debug_addr.
On Fri, 2018-06-08 at 11:55 +0200, Mark Wielaard wrote: > When constructing a "fake" Dwarf_Attribute for DW_OP_GNU_const_index, > DW_OP_constx, DW_OP_GNU_addr_index or DW_OP_addrx, we would create a > fake attribute pointing to the actual data in the .debug_addr section. > > We would even do that if there was no .debug_addr section assuming > dwarf_formaddr or dwarf_formudata would generate an error. But when > there is no .debug_addr there is also no fake_addr_cu, so the > dwarf_form* functions cannot check the value is correct (and crash). > > Fix by returning an error early from dwarf_getlocation_attr indicating > bad DWARF data. Pushed to master.
[PATCH] libdw: Break long or circular DIE ref chains in dwarf_[has]attr_integrate.
Bad DWARF could create a very long or circular DIE ref chain by linking DW_AT_abstract_origin or DW_AT_specification to the DIE itself. Break the chain after seeing a large number (16) of DIEs. Signed-off-by: Mark Wielaard --- libdw/ChangeLog | 6 ++ libdw/dwarf_attr_integrate.c| 4 ++-- libdw/dwarf_hasattr_integrate.c | 4 ++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index 1195cf6e..07a1346b 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,9 @@ +2018-06-10 Mark Wielaard + + * dwarf_attr_integrate.c (dwarf_attr_integrate): Stop after 16 DIE + ref chains. + * dwarf_hasattr_integrate.c (dwarf_hasattr_integrate): Likewise. + 2018-06-08 Mark Wielaard * dwarf_getabbrev.c (dwarf_getabbrev): Check die and offset. diff --git a/libdw/dwarf_attr_integrate.c b/libdw/dwarf_attr_integrate.c index 748d988d..fc068638 100644 --- a/libdw/dwarf_attr_integrate.c +++ b/libdw/dwarf_attr_integrate.c @@ -38,7 +38,7 @@ dwarf_attr_integrate (Dwarf_Die *die, unsigned int search_name, Dwarf_Attribute *result) { Dwarf_Die die_mem; - + int chain = 16; /* Largest DIE ref chain we will follow. */ do { Dwarf_Attribute *attr = INTUSE(dwarf_attr) (die, search_name, result); @@ -53,7 +53,7 @@ dwarf_attr_integrate (Dwarf_Die *die, unsigned int search_name, die = INTUSE(dwarf_formref_die) (attr, &die_mem); } - while (die != NULL); + while (die != NULL && chain-- != 0); /* Not NULL if it didn't have abstract_origin and specification attributes. If it is a split CU then see if the skeleton diff --git a/libdw/dwarf_hasattr_integrate.c b/libdw/dwarf_hasattr_integrate.c index 4d4b4c54..1d946280 100644 --- a/libdw/dwarf_hasattr_integrate.c +++ b/libdw/dwarf_hasattr_integrate.c @@ -37,7 +37,7 @@ int dwarf_hasattr_integrate (Dwarf_Die *die, unsigned int search_name) { Dwarf_Die die_mem; - + int chain = 16; /* Largest DIE ref chain we will follow. */ do { if (INTUSE(dwarf_hasattr) (die, search_name)) @@ -53,7 +53,7 @@ dwarf_hasattr_integrate (Dwarf_Die *die, unsigned int search_name) die = INTUSE(dwarf_formref_die) (attr, &die_mem); } - while (die != NULL); + while (die != NULL && chain-- != 0); /* Not NULL if it didn't have abstract_origin and specification attributes. If it is a split CU then see if the skeleton -- 2.17.0
Re: [PATCH] readelf, libdw: Handle too many directories or files in the line table better.
On Fri, Jun 08, 2018 at 04:06:29PM +0200, Mark Wielaard wrote: > The afl fuzzer found that the way we handle "too many" directories or files > in the (DWARF5 style) line table badly. In the case of eu-readelf we would > print an endless stream of "bad directory" or "bad file". Just stop printing > when the end of data is reached. In the case of dwarf_getsrclines we would > allocate a giant amount of memory, even if there was no data to actually > read in. Sanity check that the directory and file counts seem reasonable > compared to the amount of data left (assume we need at least 1 byte of > data per form describing the dirs or files). Pushed to master.
Re: [PATCH] tests: Fix cfi_debug_bias assert in varlocs.
On Fri, Jun 08, 2018 at 04:06:55PM +0200, Mark Wielaard wrote: > It is only a consistency issue if we actually have an cfi_debug and the > cfi_debug_bias is not zero (because they come from the same file as the > other debug data). Pushed to master.
Re: [PATCH] libdw: Detect bad DWARF in store_implicit_value.
On Fri, Jun 08, 2018 at 04:18:58PM +0200, Mark Wielaard wrote: > The afl fuzzer running against the varlocs test detected we didn't report > the value block of a DW_OP_implicit_value consistently when the DWARF was > bad. Although this doesn't cause a crash it might result in consumers > using dwarf_getlocation_implicit_value seeing an inconsistent block length > value. To fix this detect and report bad DWARF data earlier. Pushed to master.
[PATCH] readelf: Fix bounds check in print_form_data.
The afl fuzzer found that we did a wrong check in print_form_data when comparing the remaining bytes in the buffer to an (unsigned) value read. We were casting the value to ptrdiff_t which is a signed value and so might turn a really big unsigned value into a negative number. Since we know the difference between readendp and readp is zero or greater, we should cast the pointer difference to size_t (and unsigned type) instead before comparing with the unsigned value. Signed-off-by: Mark Wielaard --- src/ChangeLog | 5 + src/readelf.c | 14 +++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/ChangeLog b/src/ChangeLog index 8ebb5fb..6484b9a 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,8 @@ +2018-06-10 Mark Wielaard + + * readelf.c (print_form_data): Don't cast value to ptrdiff_t, cast + ptrdiff_t to size_t. + 2018-06-08 Mark Wielaard * readelf.c (print_debug_rnglists_section): Calculate max_entries diff --git a/src/readelf.c b/src/readelf.c index bbaaf96..fbda6c1 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -7880,7 +7880,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned char *readp, if (readendp - readp < 1) goto invalid_data; get_uleb128 (val, readp, readendp); - if (readendp - readp < (ptrdiff_t) val) + if ((size_t) (readendp - readp) < val) goto invalid_data; print_bytes (val, readp); readp += val; @@ -7890,7 +7890,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned char *readp, if (readendp - readp < 1) goto invalid_data; val = *readp++; - if (readendp - readp < (ptrdiff_t) val) + if ((size_t) (readendp - readp) < val) goto invalid_data; print_bytes (val, readp); readp += val; @@ -7900,7 +7900,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned char *readp, if (readendp - readp < 2) goto invalid_data; val = read_2ubyte_unaligned_inc (dbg, readp); - if (readendp - readp < (ptrdiff_t) val) + if ((size_t) (readendp - readp) < val) goto invalid_data; print_bytes (val, readp); readp += val; @@ -7910,7 +7910,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned char *readp, if (readendp - readp < 2) goto invalid_data; val = read_4ubyte_unaligned_inc (dbg, readp); - if (readendp - readp < (ptrdiff_t) val) + if ((size_t) (readendp - readp) < val) goto invalid_data; print_bytes (val, readp); readp += val; @@ -7941,7 +7941,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned char *readp, case DW_FORM_strp: case DW_FORM_line_strp: case DW_FORM_strp_sup: - if (readendp - readp < (ptrdiff_t) offset_len) + if ((size_t) (readendp - readp) < offset_len) goto invalid_data; if (offset_len == 8) val = read_8ubyte_unaligned_inc (dbg, readp); @@ -7965,7 +7965,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned char *readp, break; case DW_FORM_sec_offset: - if (readendp - readp < (ptrdiff_t) offset_len) + if ((size_t) (readendp - readp) < offset_len) goto invalid_data; if (offset_len == 8) val = read_8ubyte_unaligned_inc (dbg, readp); @@ -7988,7 +7988,7 @@ print_form_data (Dwarf *dbg, int form, const unsigned char *readp, { readp = data->d_buf + str_offsets_base + val; readendp = data->d_buf + data->d_size; - if (readendp - readp < (ptrdiff_t) offset_len) + if ((size_t) (readendp - readp) < offset_len) str = "???"; else { -- 1.8.3.1
Re: [PATCH] libdw: dwarf_get_units should handle existing failure to open Dwarf.
On Fri, 2018-06-08 at 20:45 +0200, Mark Wielaard wrote: > The other dwarf unit/cu iterators handle a NULL Dwarf handle as an > existing error and return NULL. Don't crash. Pushed to master.
Re: [PATCH] libdw: Check validity of dwarf_getabbrev arguments.
On Fri, 2018-06-08 at 20:47 +0200, Mark Wielaard wrote: > When the given Dwarf_Die was invalid we might crash and when the offset > was totally bogus we might succeed with a random abbrev. Pushed to master.
Re: [PATCH] tests: Don't assert on bad DW_OP_GNU_parameter_ref target in varlocs.
On Fri, 2018-06-08 at 21:18 +0200, Mark Wielaard wrote: > If the target of a DW_OP_GNU_parameter_ref isn't a DW_TAG_formal_parameter > that is bad data (which varlocs should error on). But it isn't an internal > consistency check (for which varlocs should assert). Pushed to master.
Re: [PATCH] readelf: Calculate max_entries instead of needed bytes (and overflowing).
On Fri, 2018-06-08 at 23:33 +0200, Mark Wielaard wrote: > The afl fuzzer found that we would overflow the needed bytes when > calculating how many index entries would fit in the .debug_loclists > and .debug_rnglists tables. To fix this just calculate the max number > of entries. If the offset entry count is larger than that, do emit > an error, but print up to max_entries of offsets (so the user can > more clearly see what is wrong with their table). Pushed to master.