PR #25804 opened by AlexLaroche
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25804
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25804.patch
h264_extradata_to_annexb() stored the new sps_size and pps_size before
growing the buffers with av_fast_realloc(). When an allocation failed,
for new extradata sent in packet side data, the filter returned ENOMEM
but kept the sizes:
- If the SPS buffer could not grow, sps_size was larger than the old
buffer, and the next IDR picture copied sps_size bytes from it: a heap
out of bounds read into the output packet.
- If the PPS buffer could not grow, the SPS buffer was freed while
sps_size was kept, and the next IDR picture copied from NULL.
Both happen when the caller keeps filtering after the error. Replicated
through the API with ASAN by failing the realloc() of the new parameter
set.
Store the sizes only after the buffers have grown, and on failure drop
the stored parameter sets with their sizes.
Tested with an ASAN build and a small API program that sends a packet with
AV_PKT_DATA_NEW_EXTRADATA holding a 1000-byte SPS (or PPS), makes the
realloc() of that size fail (-Wl,--wrap=realloc), then sends an IDR slice.
Before:
```
SPS case: heap-buffer-overflow, READ of size 1004 in count_or_copy()
0 bytes after a 57-byte region allocated by av_fast_realloc()
PPS case: SEGV on address 0x0 in count_or_copy()
```
After: the first packet fails with ENOMEM, the next one is filtered without
error, and LeakSanitizer reports nothing. The allocation failure cannot be
triggered from FATE, so no test is added.
>From 2af09e05b51a1f72adc88c2ff4d52f417751493a Mon Sep 17 00:00:00 2001
From: Alexandre Laroche <[email protected]>
Date: Fri, 6 Nov 2026 04:17:46 -0500
Subject: [PATCH] avcodec/bsf/h264_mp4toannexb: keep the SPS/PPS sizes in sync
on ENOMEM
h264_extradata_to_annexb() stored the new sps_size and pps_size before
growing the buffers with av_fast_realloc(). When an allocation failed,
for new extradata sent in packet side data, the filter returned ENOMEM
but kept the sizes:
- If the SPS buffer could not grow, sps_size was larger than the old
buffer, and the next IDR picture copied sps_size bytes from it: a heap
out of bounds read into the output packet.
- If the PPS buffer could not grow, the SPS buffer was freed while
sps_size was kept, and the next IDR picture copied from NULL.
Both happen when the caller keeps filtering after the error. Replicated
through the API with ASAN by failing the realloc() of the new parameter
set.
Store the sizes only after the buffers have grown, and on failure drop
the stored parameter sets with their sizes.
Signed-off-by: Alexandre Laroche <[email protected]>
---
libavcodec/bsf/h264_mp4toannexb.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/libavcodec/bsf/h264_mp4toannexb.c
b/libavcodec/bsf/h264_mp4toannexb.c
index 216fea92d17..1eacd15b4cb 100644
--- a/libavcodec/bsf/h264_mp4toannexb.c
+++ b/libavcodec/bsf/h264_mp4toannexb.c
@@ -142,13 +142,15 @@ pps:
if (pps_offset) {
uint8_t *sps;
- s->sps_size = pps_offset;
- sps = av_fast_realloc(s->sps, &s->sps_buf_size, s->sps_size);
+ sps = av_fast_realloc(s->sps, &s->sps_buf_size, pps_offset);
if (!sps) {
+ av_freep(&s->sps);
+ s->sps_size = 0;
av_free(out);
return AVERROR(ENOMEM);
}
s->sps = sps;
+ s->sps_size = pps_offset;
memcpy(s->sps, out, s->sps_size);
} else {
av_log(ctx, AV_LOG_WARNING,
@@ -158,14 +160,17 @@ pps:
if (pps_offset < total_size) {
uint8_t *pps;
- s->pps_size = total_size - pps_offset;
- pps = av_fast_realloc(s->pps, &s->pps_buf_size, s->pps_size);
+ pps = av_fast_realloc(s->pps, &s->pps_buf_size, total_size -
pps_offset);
if (!pps) {
av_freep(&s->sps);
+ av_freep(&s->pps);
+ s->sps_size = s->sps_buf_size = 0;
+ s->pps_size = 0;
av_free(out);
return AVERROR(ENOMEM);
}
s->pps = pps;
+ s->pps_size = total_size - pps_offset;
memcpy(s->pps, out + pps_offset, s->pps_size);
} else {
av_log(ctx, AV_LOG_WARNING,
--
2.54.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
