[FFmpeg-devel] [PATCH] Fix possible SIGFPEs with bad mov timings. Based on chromium patch 35_mov_bad_timings.patch

[Albert J. Wong (王重傑)]

On Thu, Nov 19, 2009 at 1:15 PM, Baptiste Coudurier <
baptiste.coudurier at gmail.com> wrote:

> On 11/19/2009 01:10 PM, Albert J. Wong (???) wrote:
>
>> On Thu, Nov 19, 2009 at 11:52 AM, Ronald S. Bultje<rsbultje at gmail.com
>> >wrote:
>>
>>  Hi,
>>>
>>> On Thu, Nov 19, 2009 at 2:37 PM, Albert J. Wong (???)
>>> <ajwong at chromium.org>  wrote:
>>>
>>>> How about something like
>>>>
>>>> if (c->time_scale<  0)
>>>>  return AVERROR_INVALIDDATA;
>>>> else if (!c->time_scale)
>>>>  c->time_scale = 1;
>>>> if (sc->time_scale<=0) {
>>>>   // the log message whatever that was...
>>>>  sc_time_scape = c->time_scale;
>>>> }
>>>>
>>>> ?
>>>>
>>>> That lets us handle the raw case, and chooses to ignore the potentially
>>>> negative values.  Is there's a better middle ground that I'm missing?
>>>>
>>>
>>> Putting this in any (=>  every) demuxer is silly, imo.
>>>
>>>
>>>  What's a better solution then?
>>
>
> Handling this problem in av_set_pts_info, I think.


What about the other places where time_scale is used in an av_rescale, or
used in regular math.  Here's a few lines of code:

mov.c:1509            st->codec->bit_rate =
stream_size*8*sc->time_scale/st->duration;

mov.c:1638  st->codec->frame_size = av_rescale(sc->stts_data[0].duration,
mov.c:1639
st->codec->sample_rate, sc->time_scale);

mov.c:2167        int64_t dts = av_rescale(current_sample->timestamp,
AV_TIME_BASE, msc->time_scale);


I'm not familiar enough with the code to fully understand what would happen
if the bitrate ended up being negative, or the frame_size ended up being
negative (or a really big unsigned), but that kind of behavior has a
tendency to feel like a pot of security or crashes waiting to be found.

-Albert