On 11/26/2019 4:29 PM, Paul B Mahol wrote:
> On 11/26/19, James Almer <jamr...@gmail.com> wrote:
>> On 11/26/2019 6:47 AM, Paul B Mahol wrote:
>>> On 11/25/19, Tomas Härdin <tjop...@acc.umu.se> wrote:
>>>> mån 2019-11-25 klockan 22:09 +0100 skrev Paul B Mahol:
>>>>> Signed-off-by: Paul B Mahol <one...@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@ffmpeg.org
>>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>>
>>>> To unsubscribe, visit link above, or email
>>>> ffmpeg-devel-requ...@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.
>
> I'm not that guy doing such work. Please stop bikesheding those
> patches for once.
>
>>
>> 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.
>
> No decoder does this.
Most decoders call av_image_check_size2() directly or indirectly to set
dimensions, which does w*h overflow checks similar to the one above.
>
>>
>>>
>>> Applied.
>>> _______________________________________________
>>> 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".
>>>
>>
>> _______________________________________________
>> 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".
> _______________________________________________
> 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".
>
_______________________________________________
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".
