[FFmpeg-devel] [PATCH 3/4] avformat/rmdec: Use 64bit for intermediate for DEINT_ID_INT4

[Andreas Rheinhardt]

Michael Niedermayer:
> Fixes: runtime error: signed integer overflow: 65312 * 65535 cannot be represented in type 'int'
> Fixes: 32832/clusterfuzz-testcase-minimized-ffmpeg_dem_RM_fuzzer-4817710040088576
> 
> Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> ---
>  libavformat/rmdec.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/rmdec.c b/libavformat/rmdec.c
> index fc3bff4859..af032ed90a 100644
> --- a/libavformat/rmdec.c
> +++ b/libavformat/rmdec.c
> @@ -269,9 +269,9 @@ static int rm_read_audio_stream_info(AVFormatContext *s, AVIOContext *pb,
>          case DEINT_ID_INT4:
>              if (ast->coded_framesize > ast->audio_framesize ||
>                  sub_packet_h <= 1 ||
> -                ast->coded_framesize * sub_packet_h > (2 + (sub_packet_h & 1)) * ast->audio_framesize)
> +                ast->coded_framesize * (uint64_t)sub_packet_h > (2 + (sub_packet_h & 1)) * ast->audio_framesize)
>                  return AVERROR_INVALIDDATA;
> -            if (ast->coded_framesize * sub_packet_h != 2*ast->audio_framesize) {
> +            if (ast->coded_framesize * (uint64_t)sub_packet_h != 2*ast->audio_framesize) {
>                  avpriv_request_sample(s, "mismatching interleaver parameters");
>                  return AVERROR_INVALIDDATA;
>              }
> 
When I looked at Real-in-Matroska, I found the checks to be not strict
enough and one of the commits that fixed this is
bdaa98dd4aac08b8f23f959cbb5a80db2dacd14a. It disallowed sub_packet_h
being odd because otherwise one could output uninitialized data. After
all, when using INT4 deiniterleavement this demuxer reads h/2 (rounded
down) blocks of size coded_framesize each in each ff_rm_parse_packet();
and after it has done this sub_packet_h times, it outputs sub_packet_h *
audio_framesize / block_align packets of size block_align each. For
RA28.8 using INT4 deinterleavement block_align == coded_framesize. So
RA28.8 using INT4 with sub_packet_h == 3, coded_framesize == 2 and
audio_framesize == 3 will pass all checks in rmdec.c as well as all
proposed checks, yet it will allocate a packet of size 3 * 3, fill 3/2 *
2 * 3 bytes of it and output 3 * 3 / 2 = 4 packets of size 2. It is
obvious that the culprit for this is h/2 being rounded down.

But there is something here which makes this more complicated than
Matroska: The Matroska demuxer simply presumes that RA288 uses INT4
deinterleavement*; yet this seems to be not guaranteed here (there is
just a comment that INT4 is the interleavement for 28.8). Forbidding odd
sub_packet_h will (together with the current checks) make sure that we
are reading sub_packet_h/2 * sub_packet_h * coded_framesize =
sub_packet_h/2 * 2 * audio_framesize = sub_packet_h * audio_framesize,
i.e. the whole packet allocated will be initialized. But if block_align
does not divide sub_packet_h * audio_framesize, then a part of the data
read will be ignored. I don't know if this can legitimately happen; it
can't happen for RA28.8.

- Andreas

*: I don't even know whether this is required by the specs (which I have
never ever seen).