[binutils-gdb] sframe fre sanity checks

[Alan Modra]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=5fab123e2090cda6862f400a5e001a9a051a765f

commit 5fab123e2090cda6862f400a5e001a9a051a765f
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Jan 20 10:27:45 2026 +1030

    sframe fre sanity checks
    
    I noticed the fre esz check in flip_sframe_fdes_with_fres_* was wrong,
    testing against the full buffer size rather than the remaining size.
    It is also ineffective at stopping buffer overflows to check after the
    buffer accesses have occurred.
    
    Likely many more buffer overflow checks in the sframe code are needed
    before anyone can claim it is secure.  Even in the fre code, I see
    things like sframe_decoder_get_fres_buf merrily iterating over fres
    without a concern for buffer overflow.
    
            * sframe.c (flip_fre): Add fp_size param.  Use it to avoid
            buffer overflow on fuzzed input.
            (flip_sframe_fdes_with_fres_v2): Pass remaining buffer size to
            flip_fre.  Remove now redundant and wrong esz check.
            (flip_sframe_fdes_with_fres_v3): Likewise.

Diff:
---
 libsframe/sframe.c | 31 +++++++++++++++++--------------
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/libsframe/sframe.c b/libsframe/sframe.c
index e915d363ca2..a0d4a408be0 100644
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ -657,35 +657,44 @@ sframe_decode_fde_attr_v3 (const char *buf, size_t buf_size,
   *fre_type = SFRAME_V3_FDE_FRE_TYPE (fdap->sfda_func_info);
   return 0;
 }
+
 static int
-flip_fre (char *fp, uint32_t fre_type, size_t *fre_size)
+flip_fre (char *fp, size_t fp_size, uint32_t fre_type, size_t *fre_size)
 {
   uint8_t fre_info;
   uint8_t offset_size, offset_cnt;
-  size_t addr_size, fre_info_size = 0;
+  size_t addr_size, fre_info_size, offset_bytes_size;
   int err = 0;
 
   if (fre_size == NULL)
     return sframe_set_errno (&err, SFRAME_ERR_INVAL);
 
+  addr_size = sframe_fre_start_addr_size (fre_type);
+  if (addr_size > fp_size)
+    return SFRAME_ERR;
   flip_fre_start_address (fp, fre_type);
 
   /* Advance the buffer pointer to where the FRE info is.  */
-  addr_size = sframe_fre_start_addr_size (fre_type);
   fp += addr_size;
+  fp_size -= addr_size;
 
   /* FRE info is uint8_t.  No need to flip.  */
+  fre_info_size = sizeof (uint8_t);
+  if (fre_info_size > fp_size)
+    return SFRAME_ERR;
   fre_info = *(uint8_t*)fp;
   offset_size = sframe_fre_get_offset_size (fre_info);
   offset_cnt = sframe_fre_get_offset_count (fre_info);
 
   /* Advance the buffer pointer to where the stack offsets are.  */
-  fre_info_size = sizeof (uint8_t);
   fp += fre_info_size;
+  fp_size -= fre_info_size;
+  offset_bytes_size = sframe_fre_offset_bytes_size (fre_info);
+  if (offset_bytes_size > fp_size)
+    return SFRAME_ERR;
   flip_fre_stack_offsets (fp, offset_size, offset_cnt);
 
-  *fre_size
-    = addr_size + fre_info_size + sframe_fre_offset_bytes_size (fre_info);
+  *fre_size = addr_size + fre_info_size + offset_bytes_size;
 
   return 0;
 }
@@ -758,12 +767,9 @@ 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 (flip_fre (fp, fre_type, &esz))
+	  if (flip_fre (fp, buf_end - fp, fre_type, &esz))
 	    goto bad;
 	  fre_bytes_flipped += esz;
-
-	  if (esz == 0 || esz > buf_size)
-	    goto bad;
 	  fp += esz;
 	}
       prev_frep_index = j;
@@ -870,12 +876,9 @@ 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 (flip_fre (fp, fre_type, &esz))
+	  if (flip_fre (fp, buf_end - fp, fre_type, &esz))
 	    goto bad;
 	  fre_bytes_flipped += esz;
-
-	  if (esz == 0 || esz > buf_size)
-	    goto bad;
 	  fp += esz;
 	}
       prev_frep_index = j;