On Wed, Feb 16, 2022 at 04:40:16PM +0800, Jiasheng Jiang wrote:
> As the potential failure of the memory allocation,
> the avformat_new_stream() could return NULL pointer.
> Therefore, it should be better to check it and return
> error if fails.
> Also, the caller, nut_read_header(), needs to deal with
> the return value of the decode_main_header() and return
> error if memory allocation fails.
> To avoid mishandling the invalid 'time_base_count', another
> check for the 'time_base_count' is needed and return different
> error if fails.
>
> Fixes: 619d8e2e58 ("updating nut demuxer to latest spec no muxing yet no
> index yet no seeking yet libnuts crcs dont match mine (didnt investigate yet)
> samplerate is stored wrong by libnut (demuxer has a workaround) code is not
> clean or beautifull yet, but i thought its better to commit early before
> someone unneccesarily wastes his time duplicating the work demuxer split from
> muxer")
> Signed-off-by: Jiasheng Jiang <jiash...@iscas.ac.cn>
> ---
> Changelog:
>
> v1 -> v2
>
> * Change 1. Add the error handling for ENOMEM from decode_main_header()
> in nut_read_header().
> * Change 2. Check for the 'time_base_count'.
> ---
> libavformat/nutdec.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/nutdec.c b/libavformat/nutdec.c
> index 0a8a700acf..4cbccb20d9 100644
> --- a/libavformat/nutdec.c
> +++ b/libavformat/nutdec.c
> @@ -220,6 +220,10 @@ static int decode_main_header(NUTContext *nut)
> }
>
> GET_V(nut->time_base_count, tmp > 0 && tmp < INT_MAX /
> sizeof(AVRational) && tmp < length/2);
> +
> + if (nut->time_base_count > NUT_MAX_STREAMS)
> + return AVERROR_INVALIDDATA;
the code already checks against length/2. If you want to add to that
that should be done at the same level and
such a change should explain why the existing check is insufficent as
well as why the new is correct
and it should be in a patch seperate from other changes
also a file with NUT_MAX_STREAMS streams could use more timebases in principle
timebases need a lot less space than streams so they could have a slightly
higher limit
> +
> nut->time_base = av_malloc_array(nut->time_base_count,
> sizeof(AVRational));
> if (!nut->time_base)
> return AVERROR(ENOMEM);
> @@ -351,8 +355,13 @@ static int decode_main_header(NUTContext *nut)
> ret = AVERROR(ENOMEM);
> goto fail;
> }
> - for (i = 0; i < stream_count; i++)
> - avformat_new_stream(s, NULL);
> + for (i = 0; i < stream_count; i++) {
> + if (!avformat_new_stream(s, NULL)) {
> + av_free(nut->stream);
freeing something and not clearing the pointer is a bad idea in general
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
