[FFmpeg-devel] [PATCH 1/2] avcodec: add mvdv video decoder

[James Almer]

On 11/26/2019 6:47 AM, Paul B Mahol wrote:
> On 11/25/19, Tomas Härdin <tjoppen at acc.umu.se> wrote:
>> mån 2019-11-25 klockan 22:09 +0100 skrev Paul B Mahol:
>>> Signed-off-by: Paul B Mahol <onemda at gmail.com>
>>> +static int decode_mvdv(MidiVidContext *s, AVCodecContext *avctx, AVFrame
>>> *frame)
>>> +{
>>> +    GetByteContext *gb = &s->gb;
>>> +    GetBitContext mask;
>>> +    GetByteContext idx9;
>>> +    uint16_t nb_vectors, intra_flag;
>>> +    const uint8_t *vec;
>>> +    const uint8_t *mask_start;
>>> +    uint8_t *skip;
>>> +    int mask_size;
>>> +    int idx9bits = 0;
>>> +    int idx9val = 0;
>>> +    int num_blocks;
>>> +
>>> +    nb_vectors = bytestream2_get_le16(gb);
>>> +    intra_flag = bytestream2_get_le16(gb);
>>> +    if (intra_flag) {
>>> +        num_blocks = (avctx->width / 2) * (avctx->height / 2);
>>
>> Will UB if width*height/4 > INT_MAX
>>
>>> +    } else {
>>> +        int skip_linesize;
>>> +
>>> +        num_blocks = bytestream2_get_le32(gb);
>>
>> Might want to use uint32_t so this doesn't lead to weirdness on 32-bit
>>
>>> +        skip_linesize = avctx->width >> 1;
>>> +        mask_start = gb->buffer_start + bytestream2_tell(gb);
>>> +        mask_size = (avctx->width >> 5) * (avctx->height >> 2);
>>
>> This can also UB
>>
>> /Tomas
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 
> Nothing of this can actually happen.

It can and i'm fairly sure it will happen as soon as the fuzzer starts
testing this decoder using big dimensions.

You don't need asserts here, you just need to check the calculations
will not overflow. Do something like "if ((int64_t)avctx->width *
avctx->height / 4 > INT_MAX) return AVERROR_INVALIDDATA" and call it a day.
Also, maybe num_blocks should be unsigned, seeing you set it using
bytestream2_get_le32() for P-frames.

> 
> Applied.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>