[FFmpeg-devel] [PATCH] Revert "avformat/mov: disallow a zero sample size in trun atoms"

[Marton Balint]

On Sun, 4 Dec 2022, Chris Ribble wrote:

>> More strict enforcement of sample size was introduced to avoid DOS/Timeout
>> with crafted (fuzzed) files and disallow emitting zero sized packets.
>>
>> Invalid file support is not something that is always worth doing, there
>> are other, more important factors, like limiting code complexity or
>> improving resiliance against denial of service. The problem here is that I
>> honestly don't know if a zero sample size is against spec, just stupid, or
>> there is a legitimate use for it.
>>
>> So I sent a 2 patch series which fixes the original issue differently.
>> Please test and review them if you can.
>
> Marton,
>
> Thank you for looking into this further.
>
> I tried your patch series and FFmpeg still generates an error
> (AVERROR_INVALIDDATA) while processing the moof fragment with
> zero-sized samples.

Hmm, strange.

>
> It seems like !(flags & MOV_TRUN_SAMPLE_SIZE) evaluates to true when
> the flag is set (512 is "inverted" to 1). Is that what you had in
> mind?

!(flags & 0x200) should evaluate to 0 if the flag is set, 1 otherwise.

>
> If I change it to this, things work as expected for my input mp4:
> if (entries && !frag->size && (flags & MOV_TRUN_SAMPLE_SIZE !=
> MOV_TRUN_SAMPLE_SIZE))

The precedence is funny here. != is evaluated first, not &.

>  return AVERROR_INVALIDDATA;
>
> Sorry if I was supposed to provide this feedback on the patch series
> itself; I'm happy to do so there as needed.

Share the sample please. If the patch I proposed does not fix it, I am not 
sure what is the best approach here.

Thanks,
Marton