diff mbox series

[09/10] libsframe: validate bounds before reads during endian flipping

Message ID 20260827224146.3391610-10-ibhagatgnu@gmail.com
State New
Headers
Series fix two PRs and guard against bad data |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

Indu Bhagat Aug. 27, 2026, 10:41 p.m. UTC 
  Currently, in both flip_sframe_fdes_with_fres_v2 () and
flip_sframe_fdes_with_fres_v3 (), the pointer fp to a location in the
SFrame FRE subsection is calculated using the unvalidated offset
fre_offset read from the SFrame section:

    fp = fres + fre_offset

If an invalid or malicious SFrame binary contains a large fre_offset
(sfh_freoff is already validated in sframe_header_sanity_check_p), fp
may point past the buf_end.

Validate fp pointers in flip_sframe_fdes_with_fres_v2 () and
flip_sframe_fdes_with_fres_v3 () before calling the code to flip FREs.
While at it, also address similar issue with fdes location access.

This addresses some of the issues raised in PR libsframe/34273.
---
 libsframe/sframe.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Jens Remus Sept. 7, 2026, 1:07 p.m. UTC | #1 
On 8/28/2026 12:41 AM, Indu Bhagat wrote:
> Currently, in both flip_sframe_fdes_with_fres_v2 () and
> flip_sframe_fdes_with_fres_v3 (), the pointer fp to a location in the
> SFrame FRE subsection is calculated using the unvalidated offset
> fre_offset read from the SFrame section:
> 
>     fp = fres + fre_offset
> 
> If an invalid or malicious SFrame binary contains a large fre_offset
> (sfh_freoff is already validated in sframe_header_sanity_check_p), fp
> may point past the buf_end.
> 
> Validate fp pointers in flip_sframe_fdes_with_fres_v2 () and
> flip_sframe_fdes_with_fres_v3 () before calling the code to flip FREs.
> While at it, also address similar issue with fdes location access.
> 
> This addresses some of the issues raised in PR libsframe/34273.
> ---
>  libsframe/sframe.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Jens Remus <jremus@linux.ibm.com>

One question below though?

> diff --git a/libsframe/sframe.c b/libsframe/sframe.c

> @@ -794,7 +794,7 @@ flip_sframe_fdes_with_fres_v2 (char *frame_buf, size_t buf_size,
>    size_t fsz = sizeof (sframe_func_desc_entry_v2);
>    for (i = 0; i < num_fdes; fdes += fsz, i++)
>      {
> -      if (fdes >= buf_end)
> +      if (fdes >= buf_end || (size_t)(buf_end - fdes) < fsz)
>  	goto bad;
>  
>        /* Handle FDE.  */

Wouldn't the following work?  Or could fp wrap around due to fp += esz?

> @@ -817,6 +817,8 @@ flip_sframe_fdes_with_fres_v2 (char *frame_buf, size_t buf_size,

      if (fre_offset > ihp->sfh_fre_len)
        goto bad;
>        fp = fres + fre_offset;
>        for (; j < prev_frep_index + num_fres; j++)
>  	{
> +	  if (fp < fres || fp >= buf_end)
> +	    goto bad;

          if (fp >= buf_end)
            goto bad;

>  	  if (flip_fre (fp, buf_end - fp, fre_type, &esz))
>  	    goto bad;
>  	  fre_bytes_flipped += esz;
> @@ -890,7 +892,7 @@ flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
>    size_t fsz = sizeof (sframe_func_desc_idx_v3);
>    for (i = 0; i < num_fdes; fdes += fsz, i++)
>      {
> -      if (fdes >= buf_end)
> +      if (fdes >= buf_end || (size_t)(buf_end - fdes) < fsz)
>  	goto bad;
>  
>        /* Handle FDE.  */
> @@ -909,6 +911,10 @@ flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
>  
>        /* Handle FDE attr (only in V3).  */
>        fp = fres + fre_offset;
> +      if (fp < fres || fp >= buf_end
> +	  || (size_t)(buf_end - fp) < sizeof (sframe_func_desc_attr_v3))
> +	goto bad;

Likewise.

> +
>        if (to_foreign && sframe_decode_fde_attr_v3 (fp, buf_end - fp,
>  						   &num_fres, &fre_type))
>  	goto bad;
> @@ -926,6 +932,8 @@ flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
>        fp += sizeof (sframe_func_desc_attr_v3);
>        for (; j < prev_frep_index + num_fres; j++)
>  	{
> +	  if (fp < fres || fp >= buf_end)
> +	    goto bad;

Likewise.

>  	  if (flip_fre (fp, buf_end - fp, fre_type, &esz))
>  	    goto bad;
>  	  fre_bytes_flipped += esz;

Regards,
Jens
diff mbox series

Patch

diff --git a/libsframe/sframe.c b/libsframe/sframe.c
index 5ddd3962212..d3af9717fe2 100644
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ -794,7 +794,7 @@  flip_sframe_fdes_with_fres_v2 (char *frame_buf, size_t buf_size,
   size_t fsz = sizeof (sframe_func_desc_entry_v2);
   for (i = 0; i < num_fdes; fdes += fsz, i++)
     {
-      if (fdes >= buf_end)
+      if (fdes >= buf_end || (size_t)(buf_end - fdes) < fsz)
 	goto bad;
 
       /* Handle FDE.  */
@@ -817,6 +817,8 @@  flip_sframe_fdes_with_fres_v2 (char *frame_buf, size_t buf_size,
       fp = fres + fre_offset;
       for (; j < prev_frep_index + num_fres; j++)
 	{
+	  if (fp < fres || fp >= buf_end)
+	    goto bad;
 	  if (flip_fre (fp, buf_end - fp, fre_type, &esz))
 	    goto bad;
 	  fre_bytes_flipped += esz;
@@ -890,7 +892,7 @@  flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
   size_t fsz = sizeof (sframe_func_desc_idx_v3);
   for (i = 0; i < num_fdes; fdes += fsz, i++)
     {
-      if (fdes >= buf_end)
+      if (fdes >= buf_end || (size_t)(buf_end - fdes) < fsz)
 	goto bad;
 
       /* Handle FDE.  */
@@ -909,6 +911,10 @@  flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
 
       /* Handle FDE attr (only in V3).  */
       fp = fres + fre_offset;
+      if (fp < fres || fp >= buf_end
+	  || (size_t)(buf_end - fp) < sizeof (sframe_func_desc_attr_v3))
+	goto bad;
+
       if (to_foreign && sframe_decode_fde_attr_v3 (fp, buf_end - fp,
 						   &num_fres, &fre_type))
 	goto bad;
@@ -926,6 +932,8 @@  flip_sframe_fdes_with_fres_v3 (char *frame_buf, size_t buf_size,
       fp += sizeof (sframe_func_desc_attr_v3);
       for (; j < prev_frep_index + num_fres; j++)
 	{
+	  if (fp < fres || fp >= buf_end)
+	    goto bad;
 	  if (flip_fre (fp, buf_end - fp, fre_type, &esz))
 	    goto bad;
 	  fre_bytes_flipped += esz;