[FFmpeg-devel] [PATCH 2/5] avformat/matroskaenc: fix invalid pointer access if avio_get_dyn_buf failed

[Andreas Rheinhardt]

lance.lmwang at gmail.com:
> From: Limin Wang <lance.lmwang at gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> ---
>  libavformat/matroskaenc.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 784973a951..f0474da44f 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -374,9 +374,12 @@ static void end_ebml_master_crc32(AVIOContext *pb, AVIOContext **dyn_cp,
>      put_ebml_length(pb, size, length_size);
>      if (mkv->write_crc) {
>          skip = 6; /* Skip reserved 6-byte long void element from the dynamic buffer. */
> +        if (size > skip) {
>          AV_WL32(crc, av_crc(av_crc_get_table(AV_CRC_32_IEEE_LE), UINT32_MAX, buf + skip, size - skip) ^ UINT32_MAX);
>          put_ebml_binary(pb, EBML_ID_CRC32, crc, sizeof(crc));
> +        }
>      }
> +    if (size > skip)
>      avio_write(pb, buf + skip, size - skip);
>  
>      if (keep_buffer) {
> 
I sent a patch containing proper checks for this and other allocations
in this muxer here [1].

- Andreas

PS: avio_close_dyn_buf() is even worse: Besides the design flaw of
freeing a resource without setting the pointer to it to NULL, it returns
a size of -AV_INPUT_BUFFER_PADDING_SIZE if a memory allocation failure
happened (but not if the arbitrary limit of INT_MAX/2 has been
surpassed); and this despite its documentation not allowing returning
negative values at all. (And it returns the current position of the
write pointer as size and zeroes what comes immediately after, even if a
seek to the front has happened.)

[1]: http://ffmpeg.org/pipermail/ffmpeg-devel/2020-April/261704.html