[FFmpeg-devel] [PATCH 2/5] avcodec/cbs_mpeg2: Change assertion from bytes to bits

[James Almer]

On 12/8/2019 6:31 PM, Michael Niedermayer wrote:
> This allows writing empty slices

Are empty slices valid in mpeg2 to begin with? Or is that the result of
invalid/truncated data exclusively?

The code in cbs_mpeg2_write_slice() is run only when slice->data is not
NULL, which after a cursory look at cbs_mpeg2_read_slice() it looks like
is always the case (It will always have a pointer, even if it points to
the very end of the buffer). In contrast, slice->data_size can be zero,
so maybe the check here should be slice->data_size != 0 instead, which
should fix this assertion failure without changing the it or what data
the module will accept.

> 
> Fixes: Assertion slice->data_bit_start >= 0 && slice->data_size > slice->data_bit_start / 8 failed at libavcodec/cbs_mpeg2.c:340
> Fixes: 19280/clusterfuzz-testcase-minimized-ffmpeg_BSF_MPEG2_METADATA_fuzzer-5632206173372416
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavcodec/cbs_mpeg2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c
> index 13d871cc89..b5286537db 100644
> --- a/libavcodec/cbs_mpeg2.c
> +++ b/libavcodec/cbs_mpeg2.c
> @@ -337,7 +337,7 @@ static int cbs_mpeg2_write_slice(CodedBitstreamContext *ctx,
>          uint8_t *pos = slice->data + slice->data_bit_start / 8;
>  
>          av_assert0(slice->data_bit_start >= 0 &&
> -                   slice->data_size > slice->data_bit_start / 8);
> +                   slice->data_size * 8LL >= slice->data_bit_start);
>  
>          if (slice->data_size * 8 + 8 > put_bits_left(pbc))
>              return AVERROR(ENOSPC);
>