public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] libdw, readelf, elflint: Add get_(u|s)leb128 guards
@ 2023-02-12 15:27 Mark Wielaard
  0 siblings, 0 replies; only message in thread
From: Mark Wielaard @ 2023-02-12 15:27 UTC (permalink / raw)
  To: elfutils-devel; +Cc: vvvvvv, Mark Wielaard

Add sanity check making sure an leb128 isn't being read beyond the
end of the current data segment. Most code already had these guards,
but some were missing. This makes sure an appropriate error is
generated instead.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libdw/ChangeLog              | 16 ++++++++++++++++
 libdw/cfi.c                  |  2 ++
 libdw/dwarf_child.c          |  3 +++
 libdw/dwarf_frame_register.c |  5 +++++
 libdw/dwarf_getabbrev.c      |  1 +
 libdw/dwarf_getlocation.c    |  2 +-
 libdw/dwarf_getsrclines.c    |  2 ++
 libdw/encoded-value.h        |  4 ++++
 libdw/fde.c                  |  3 +++
 libdw/libdw_form.c           |  6 ++++++
 src/ChangeLog                | 12 ++++++++++++
 src/elflint.c                |  3 +++
 src/readelf.c                | 14 ++++++++++++++
 13 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/libdw/ChangeLog b/libdw/ChangeLog
index 71e96c88..2c1f61e8 100644
--- a/libdw/ChangeLog
+++ b/libdw/ChangeLog
@@ -1,3 +1,19 @@
+2023-02-12  Mark Wielaard  <mark@klomp.org>
+
+	* cfi.c (execute_cfi): Add cfi_asser before reading second lib128.
+	* dwarf_chld.c (__libdw_find_attr): Check readp >= endp before
+	calling get_uleb128.
+	* dwarf_frame_register.c (dwarf_frame_register): Likewise for
+	p >= end.
+	* dwarf_getabbrev.c (__libdw_getabbrev): Add comment about check.
+	* dwarf_getlocation.c (__libdw_intern_expression): Update check to
+	account for both the number and uleb128.
+	* encoded-value.h (read_encoded_value): Check p >= end for
+	DW_EH_PE_(u|s)leb128.
+	* fde.c (intern_fde): Check len can be read as uleb128.
+	* libdw_form.c (__libdw_form_val_compute_len): Check valp >= endp
+	before get_uleb128.
+
 2023-01-22  Mark Wielaard  <mark@klomp.org>
 
 	* dwarf_getscopes.c (pc_record): Return nscopes when done.
diff --git a/libdw/cfi.c b/libdw/cfi.c
index 6d08ca90..a7174405 100644
--- a/libdw/cfi.c
+++ b/libdw/cfi.c
@@ -239,6 +239,7 @@ execute_cfi (Dwarf_CFI *cache,
 
 	case DW_CFA_offset_extended_sf:
 	  get_uleb128 (operand, program, end);
+	  cfi_assert (program < end);
 	  get_sleb128 (sf_offset, program, end);
 	offset_extended_sf:
 	  offset = sf_offset * cie->data_alignment_factor;
@@ -294,6 +295,7 @@ execute_cfi (Dwarf_CFI *cache,
 	  get_uleb128 (regno, program, end);
 	  /* DW_FORM_block is a ULEB128 length followed by that many bytes.  */
 	  offset = program - (const uint8_t *) cache->data->d.d_buf;
+	  cfi_assert (program < end);
 	  get_uleb128 (operand, program, end);
 	  cfi_assert (operand <= (Dwarf_Word) (end - program));
 	  program += operand;
diff --git a/libdw/dwarf_child.c b/libdw/dwarf_child.c
index c8c8bb61..96a0d608 100644
--- a/libdw/dwarf_child.c
+++ b/libdw/dwarf_child.c
@@ -73,10 +73,13 @@ __libdw_find_attr (Dwarf_Die *die, unsigned int search_name,
 
       if (attr_form == DW_FORM_indirect)
 	{
+	  if (readp >= endp)
+	    goto invalid;
 	  get_uleb128 (attr_form, readp, endp);
 	  if (attr_form == DW_FORM_indirect ||
 	      attr_form == DW_FORM_implicit_const)
 	    {
+	    invalid:
 	      __libdw_seterrno (DWARF_E_INVALID_DWARF);
 	      return NULL;
 	    }
diff --git a/libdw/dwarf_frame_register.c b/libdw/dwarf_frame_register.c
index bcf3fa03..a6b7c4c1 100644
--- a/libdw/dwarf_frame_register.c
+++ b/libdw/dwarf_frame_register.c
@@ -100,6 +100,11 @@ dwarf_frame_register (Dwarf_Frame *fs, int regno, Dwarf_Op ops_mem[3],
 	const uint8_t *p = fs->cache->data->d.d_buf + reg->value;
 	const uint8_t *end = (fs->cache->data->d.d_buf
 			      + fs->cache->data->d.d_size);
+	if (p >= end)
+	  {
+	    __libdw_seterrno (DWARF_E_INVALID_DWARF);
+	    return -1;
+	  }
 	get_uleb128 (block.length, p, end);
 	block.data = (void *) p;
 
diff --git a/libdw/dwarf_getabbrev.c b/libdw/dwarf_getabbrev.c
index 13bee493..5b02333f 100644
--- a/libdw/dwarf_getabbrev.c
+++ b/libdw/dwarf_getabbrev.c
@@ -77,6 +77,7 @@ __libdw_getabbrev (Dwarf *dbg, struct Dwarf_CU *cu, Dwarf_Off offset,
 			      + dbg->sectiondata[IDX_debug_abbrev]->d_size);
   const unsigned char *start_abbrevp = abbrevp;
   unsigned int code;
+  // We start off with abbrevp at offset, which is checked above.
   get_uleb128 (code, abbrevp, end);
 
   /* Check whether this code is already in the hash table.  */
diff --git a/libdw/dwarf_getlocation.c b/libdw/dwarf_getlocation.c
index d0d78163..4e8c047b 100644
--- a/libdw/dwarf_getlocation.c
+++ b/libdw/dwarf_getlocation.c
@@ -545,7 +545,7 @@ __libdw_intern_expression (Dwarf *dbg, bool other_byte_order,
 	case DW_OP_deref_type:
 	case DW_OP_GNU_deref_type:
 	case DW_OP_xderef_type:
-	  if (unlikely (data + 1 >= end_data))
+	  if (unlikely (data + 2 >= end_data))
 	    goto invalid;
 	  newloc->number = *data++;
 	  get_uleb128 (newloc->number2, data, end_data);
diff --git a/libdw/dwarf_getsrclines.c b/libdw/dwarf_getsrclines.c
index 2c1d7a40..df003c5f 100644
--- a/libdw/dwarf_getsrclines.c
+++ b/libdw/dwarf_getsrclines.c
@@ -572,6 +572,8 @@ read_srclines (Dwarf *dbg,
 	goto invalid_data;
 
       size_t nfiles;
+      if ((size_t) (lineendp - linep) < 1)
+	goto invalid_data;
       get_uleb128 (nfiles, linep, lineendp);
 
       if (nforms == 0 && nfiles != 0)
diff --git a/libdw/encoded-value.h b/libdw/encoded-value.h
index d4e01924..4566ef96 100644
--- a/libdw/encoded-value.h
+++ b/libdw/encoded-value.h
@@ -196,10 +196,14 @@ read_encoded_value (const Dwarf_CFI *cache, uint8_t encoding,
       break;
 
     case DW_EH_PE_uleb128:
+      if (*p >= endp)
+	goto invalid_data;
       get_uleb128 (value, *p, endp);
       break;
 
     case DW_EH_PE_sleb128:
+      if (*p >= endp)
+	goto invalid_data;
       get_sleb128 (value, *p, endp);
       break;
 
diff --git a/libdw/fde.c b/libdw/fde.c
index f5f6fbe1..73d551b6 100644
--- a/libdw/fde.c
+++ b/libdw/fde.c
@@ -104,9 +104,12 @@ intern_fde (Dwarf_CFI *cache, const Dwarf_FDE *entry)
       /* The CIE augmentation says the FDE has a DW_FORM_block
 	 before its actual instruction stream.  */
       Dwarf_Word len;
+      if (fde->instructions >= fde->instructions_end)
+	goto invalid;
       get_uleb128 (len, fde->instructions, fde->instructions_end);
       if ((Dwarf_Word) (fde->instructions_end - fde->instructions) < len)
 	{
+	invalid:
 	  free (fde);
 	  __libdw_seterrno (DWARF_E_INVALID_DWARF);
 	  return NULL;
diff --git a/libdw/libdw_form.c b/libdw/libdw_form.c
index c83dfb39..40045440 100644
--- a/libdw/libdw_form.c
+++ b/libdw/libdw_form.c
@@ -88,6 +88,8 @@ __libdw_form_val_compute_len (struct Dwarf_CU *cu, unsigned int form,
 
     case DW_FORM_block:
     case DW_FORM_exprloc:
+      if (valp >= endp)
+       goto invalid;
       get_uleb128 (u128, valp, endp);
       result = u128 + (valp - startp);
       break;
@@ -111,6 +113,8 @@ __libdw_form_val_compute_len (struct Dwarf_CU *cu, unsigned int form,
     case DW_FORM_strx:
     case DW_FORM_GNU_addr_index:
     case DW_FORM_GNU_str_index:
+      if (valp >= endp)
+       goto invalid;
       get_uleb128 (u128, valp, endp);
       result = valp - startp;
       break;
@@ -119,6 +123,8 @@ __libdw_form_val_compute_len (struct Dwarf_CU *cu, unsigned int form,
       /* The amount of data to skip in the DIE is the size of the actual
 	 FORM data (which is __libdw_form_val_len) plus the size of the
 	 uleb128 encoding that FORM (which is valp - startp).  */
+      if (valp >= endp)
+	goto invalid;
       get_uleb128 (u128, valp, endp);
       if (*valp == DW_FORM_indirect || *valp == DW_FORM_implicit_const)
 	return (size_t) -1;
diff --git a/src/ChangeLog b/src/ChangeLog
index 915494f2..699d98ee 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,15 @@
+2023-02-12  Mark Wielaard  <mark@klomp.org>
+
+	* readelf.c (print_attributes): Add comment about check.
+	(read_encoded): Check readp >= endp before reading
+	DW_EH_PE_uleb128 and DW_EH_PE_sleb128.
+	* elflint.c (check_attributes): Check r >= q before reading
+	uleb128.
+	(print_debug_frame_section): Check augmentation length can be read
+	as uleb128.
+	(print_debug_exception_table): Likewise for ttype_base_offset,
+	call_site_table_len and action.
+
 2023-01-22  Mark Wielaard  <mark@klomp.org>
 
 	* addr2line.c (options): Separate --demangle and -C.
diff --git a/src/elflint.c b/src/elflint.c
index b4eac32f..dd42dcb4 100644
--- a/src/elflint.c
+++ b/src/elflint.c
@@ -3569,9 +3569,12 @@ section [%2d] '%s': offset %zu: attribute subsection has unexpected tag %u\n"),
 		    const unsigned char *r = chunk;
 		    if (tag == 32 || (tag & 1) == 0)
 		      {
+			if (r >= q)
+			  goto invalid_uleb;
 			get_uleb128 (value, r, q);
 			if (r > q)
 			  {
+			  invalid_uleb:
 			    ERROR (_("\
 section [%2d] '%s': offset %zu: endless ULEB128 in attribute tag\n"),
 				   idx, section_name (ebl, idx), buffer_pos (data, chunk));
diff --git a/src/readelf.c b/src/readelf.c
index 5b3319c2..0f13874f 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -3802,6 +3802,7 @@ print_attributes (Ebl *ebl, const GElf_Ehdr *ehdr)
 			if (tag == 32 || (tag & 1) == 0
 			    || (! gnu_vendor && (tag > 5 && tag < 32)))
 			  {
+			    // Note r >= q check above.
 			    get_uleb128 (value, r, q);
 			    if (r > q)
 			      break;
@@ -6368,9 +6369,13 @@ read_encoded (unsigned int encoding, const unsigned char *readp,
   switch (encoding & 0xf)
     {
     case DW_EH_PE_uleb128:
+      if (readp >= endp)
+	goto invalid;
       get_uleb128 (*res, readp, endp);
       break;
     case DW_EH_PE_sleb128:
+      if (readp >= endp)
+	goto invalid;
       get_sleb128 (*res, readp, endp);
       break;
     case DW_EH_PE_udata2:
@@ -6983,6 +6988,9 @@ print_debug_frame_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr,
 
 	  if (augmentation[0] == 'z')
 	    {
+	      if (cieend - readp < 1)
+		goto invalid_data;
+
 	      unsigned int augmentationlen;
 	      get_uleb128 (augmentationlen, readp, cieend);
 
@@ -11010,6 +11018,8 @@ print_debug_exception_table (Dwfl_Module *dwflmod __attribute__ ((unused)),
   if (ttype_encoding != DW_EH_PE_omit)
     {
       unsigned int ttype_base_offset;
+      if (readp >= dataend)
+	goto invalid_data;
       get_uleb128 (ttype_base_offset, readp, dataend);
       printf (" TType base offset:   %#x\n", ttype_base_offset);
       if ((size_t) (dataend - readp) > ttype_base_offset)
@@ -11022,6 +11032,8 @@ print_debug_exception_table (Dwfl_Module *dwflmod __attribute__ ((unused)),
   printf (_(" Call site encoding:  %#x "), call_site_encoding);
   print_encoding_base ("", call_site_encoding);
   unsigned int call_site_table_len;
+  if (readp >= dataend)
+    goto invalid_data;
   get_uleb128 (call_site_table_len, readp, dataend);
 
   const unsigned char *const action_table = readp + call_site_table_len;
@@ -11044,6 +11056,8 @@ print_debug_exception_table (Dwfl_Module *dwflmod __attribute__ ((unused)),
       readp = read_encoded (call_site_encoding, readp, dataend,
 			    &landing_pad, dbg);
       unsigned int action;
+      if (readp >= dataend)
+	goto invalid_data;
       get_uleb128 (action, readp, dataend);
       max_action = MAX (action, max_action);
       printf (_(" [%4u] Call site start:   %#" PRIx64 "\n"
-- 
2.31.1


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-02-12 15:27 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-12 15:27 [PATCH] libdw, readelf, elflint: Add get_(u|s)leb128 guards Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).