[FFmpeg-devel] [PATCH] avcodec/h2645_parse: remove initial skipped_bytes_pos buffer

[Michael Niedermayer]

On Thu, Oct 08, 2020 at 10:25:11AM -0300, James Almer wrote:
> Allocate it only when needed, and instead of giving it a fixed initial size
> that's doubled on each realloc, ensure it's always big enough for the NAL
> currently being parsed.
> 
> Fixes: OOM
> Fixes: 23817/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_METADATA_fuzzer-6300869057576960
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: James Almer <jamrial at gmail.com>
> ---
>  libavcodec/h2645_parse.c | 28 ++++++++++------------------
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/libavcodec/h2645_parse.c b/libavcodec/h2645_parse.c
> index 0f98b49fbe..f5c76323c1 100644
> --- a/libavcodec/h2645_parse.c
> +++ b/libavcodec/h2645_parse.c
> @@ -108,22 +108,20 @@ int ff_h2645_extract_rbsp(const uint8_t *src, int length,
>                  dst[di++] = 0;
>                  si       += 3;
>  
> -                if (nal->skipped_bytes_pos) {
> -                    nal->skipped_bytes++;
> -                    if (nal->skipped_bytes_pos_size < nal->skipped_bytes) {
> -                        nal->skipped_bytes_pos_size *= 2;
> -                        av_assert0(nal->skipped_bytes_pos_size >= nal->skipped_bytes);
> -                        av_reallocp_array(&nal->skipped_bytes_pos,
> +                nal->skipped_bytes++;
> +                if (nal->skipped_bytes_pos_size < nal->skipped_bytes) {
> +                    nal->skipped_bytes_pos_size = length / 3;

This would allocate a much larger than needed skipped_bytes_pos for 
probably nearly all of the real world h264 files to fix an issue with
crafted streams.

The initial size should be choosen so as to be large enough for real world
streams. If that turns out to be too small then length /3 sounds reasonable
as the first reallocation.

thx
[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20201009/4c1aa851/attachment.sig>