Skip to content

Fix segfaults and use-after-frees - #2476

Merged
WyattBlue merged 2 commits into
masterfrom
segfault-fixes
Oct 25, 2026
Merged

WyattBlue merged 2 commits into
masterfrom
segfault-fixes

Conversation

@WyattBlue

Copy link
Copy Markdown
Member

Found by fuzzing the public API in a crash-isolating harness (objects in closed, freed, empty, and __new__ states; ~112k malformed inputs) and auditing memcpy/pointer sites.

Segfaults

  • Resuming demux()/decode() after the container closed called av_read_frame(NULL, …). Easy to hit: create the generator inside with av.open(...) and iterate after the block. Now raises AssertionError: Container is not open.
  • A Python file whose read(n) returned more than n bytes overflowed FFmpeg's I/O buffer. Now raises ValueError.
  • Inserting into an av.dictionary.Dictionary while iterating it left the iterator with a dangling entry pointer. Iteration now snapshots the keys.
  • Cls.__new__(Cls) left a NULL pointer on HWConfig and the side data wrappers, because their sentinel check lived in __init__. Moved to __cinit__, so it raises TypeError like the other wrappers.

Use-after-frees

  • Packet(bytearray) referenced the bytearray without a buffer export, so resizing it left the packet reading freed memory (also a race in CodecContext.parse). bytearray now goes through PyObject_GetBuffer; resizing it while the packet lives raises BufferError, as it already did for memoryview input.
  • VideoBlockParams didn't keep its VideoEncParams alive.

Also

  • str input to Packet, extradata, and subtitle_header dropped bytes of non-ASCII text (len() counted characters, not UTF-8 bytes). Separate commit.
  • Stubs: Packet(input) accepts int | str | Buffer | None; Dictionary(...) accepts mappings and keywords, matching runtime.

Each regression test fails on the old code (two of them by segfaulting/hanging).

- `demux()`/`decode()` resumed after the container closed read from the
  freed format context; check the container is open on every iteration
- A Python file whose `read(n)` returned more than `n` bytes overflowed
  FFmpeg's I/O buffer; raise `ValueError` instead
- `Packet(bytearray)` referenced the bytearray without a buffer export,
  so resizing it left the packet reading freed memory; only take the
  no-copy path for immutable `bytes` and `str`
- `Dictionary` iteration held an entry pointer that inserts invalidated;
  snapshot the keys first
- `VideoBlockParams` didn't keep its `VideoEncParams` alive
- `HWConfig` and the side data wrappers checked their sentinel in
  `__init__`, so `__new__` left a NULL pointer; check in `__cinit__`

Also type `Packet(input)` and `Dictionary(...)` to match what they accept.
`ByteSource` took `len()` of a str, which counts characters, so
`Packet("né")` and `CodecContext.extradata = "né"` dropped the last
byte of any non-ASCII text. Get the pointer and byte length together
from `PyUnicode_AsUTF8AndSize`; the str owns the buffer, and for ASCII
it is the str's own data, so nothing is copied.
@WyattBlue
WyattBlue merged commit 97dbf8e into master Oct 25, 2026
9 checks passed
@WyattBlue
WyattBlue deleted the segfault-fixes branch October 25, 2026 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant