[PATCH 04/10] libsframe: validate FDE/FRE offsets in SFrame header

Currently, the check for preamble magic, version, flags was done in
sframe_decode (), while sframe_header_sanity_check_p () performed
minimal sanity check of sfh_fdeoff <= sfh_freoff.

There is need to validate header section offsets and length fields
(sfh_fdeoff, sfh_freoff, sfh_fre_len, sfh_num_fdes) against the
caller-supplied buffer size buf_size. If unchecked, a crafted .sframe
section with invalid header offsets may cause out-of-bounds reads.

Update sframe_header_sanity_check_p() to accept buf_size and validate:
  - Header size hdrsz <= buf_size
  - hdrsz + sfh_fdeoff + num_fdes * fde_entry_size <= sfh_freoff
  - hdrsz + sfh_freoff + sfh_fre_len <= buf_size

Update all call sites of sframe_header_sanity_check_p() to pass the
buffer size.

This fixes a subset of issues raised in PR libsframe/34273

An alternate fix was suggested by Naveed Khan earlier on the mailing
list.  This commit is an adaptation of the same.

Suggested-by: Naveed Khan <[email protected]>
---
 libsframe/sframe.c | 43 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 37 insertions(+), 6 deletions(-)

diff --git a/libsframe/sframe.c b/libsframe/sframe.c
index d50a9e5689b..4742e4550c2 100644
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ -358,8 +358,11 @@ flip_fde_attr_v3 (char *buf, size_t buf_size)
 /* Check if SFrame header has valid data.  */
 
 static bool
-sframe_header_sanity_check_p (const sframe_header *hp)
+sframe_header_sanity_check_p (const sframe_header *hp, size_t buf_size)
 {
+  if (buf_size < sizeof (sframe_header))
+    return false;
+
   /* Check preamble is valid.  */
   if (hp->sfh_preamble.sfp_magic != SFRAME_MAGIC
       || (hp->sfh_preamble.sfp_version != SFRAME_VERSION_1
@@ -377,10 +380,38 @@ sframe_header_sanity_check_p (const sframe_header *hp)
   if (hp->sfh_preamble.sfp_flags & ~valid_flags)
     return false;
 
-  /* Check offsets are valid.  */
+  size_t hdrsz = sframe_get_hdr_size (hp);
+  if (buf_size < hdrsz)
+    return false;
+
+  /* Check offsets are within buf_size and sensible.  */
+  size_t fde_entry_size = (hp->sfh_preamble.sfp_version == SFRAME_VERSION_3)
+			   ? sizeof (sframe_func_desc_idx_v3)
+			   : sizeof (sframe_func_desc_entry_v2);
+
+  if (hp->sfh_num_fdes > (SIZE_MAX / fde_entry_size))
+    return false;
+
+  size_t fidx_size = hp->sfh_num_fdes * fde_entry_size;
+
   if (hp->sfh_fdeoff > hp->sfh_freoff)
     return false;
 
+  if (hp->sfh_fdeoff > buf_size - hdrsz)
+    return false;
+
+  if (fidx_size > buf_size - hdrsz - hp->sfh_fdeoff)
+    return false;
+
+  if (hp->sfh_freoff > buf_size - hdrsz)
+    return false;
+
+  if (hp->sfh_fre_len > buf_size - hdrsz - hp->sfh_freoff)
+    return false;
+
+  if (hp->sfh_fdeoff + fidx_size > hp->sfh_freoff)
+    return false;
+
   return true;
 }
 
@@ -733,7 +764,7 @@ flip_sframe_fdes_with_fres_v2 (char *frame_buf, size_t buf_size,
   /* Header must be in host endianness at this time.  */
   const sframe_header *ihp = (sframe_header *)frame_buf;
 
-  if (!sframe_header_sanity_check_p (ihp))
+  if (!sframe_header_sanity_check_p (ihp, buf_size))
     return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
 
   /* The contents of the SFrame header are safe to read.  Get the number of
@@ -829,7 +860,7 @@ flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
   /* Header must be in host endianness at this time.  */
   const sframe_header *ihp = (sframe_header *)frame_buf;
 
-  if (!sframe_header_sanity_check_p (ihp))
+  if (!sframe_header_sanity_check_p (ihp, buf_size))
     return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
 
   /* The contents of the SFrame header are safe to read.  Get the number of
@@ -917,7 +948,7 @@ flip_sframe (char *frame_buf, size_t buf_size, uint32_t to_foreign)
 
   /* Header must be in host endianness at this time.  */
   const sframe_header *ihp = (sframe_header *)frame_buf;
-  if (!sframe_header_sanity_check_p (ihp))
+  if (!sframe_header_sanity_check_p (ihp, buf_size))
     return sframe_set_errno (&err, SFRAME_ERR_BUF_INVAL);
   uint8_t ver = ihp->sfh_preamble.sfp_version;
 
@@ -1482,7 +1513,7 @@ sframe_decode (const char *sf_buf, size_t sf_size, int *errp)
   dctx->sfd_header = *(sframe_header *) frame_buf;
   /* Validate the contents of SFrame header.  */
   dhp = &dctx->sfd_header;
-  if (!sframe_header_sanity_check_p (dhp))
+  if (!sframe_header_sanity_check_p (dhp, sf_size))
     {
       sframe_ret_set_errno (errp, SFRAME_ERR_BUF_INVAL);
       goto decode_fail_free;
-- 
2.43.0