From: Alan Modra Date: Mon, 19 Jan 2026 23:57:45 +0000 (+1030) Subject: sframe fre sanity checks X-Git-Tag: binutils-2_46~134 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5fab123e2090cda6862f400a5e001a9a051a765f;p=thirdparty%2Fbinutils-gdb.git 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 --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;