[FFmpeg-cvslog] r17011 - trunk/libavformat/mxfenc.c

[Reimar Döffinger]

On Thu, Feb 05, 2009 at 04:33:02PM -0800, Baptiste Coudurier wrote:
> On 2/5/2009 3:46 PM, Reimar D?ffinger wrote:
> > You can't know if this is security-critical without knowing what the
> > application using ffmpeg does.
> > Fact is that it causes a side-effect which changes global state - other
> > functions like random() were forbidden for this reason.
> 
> I'm not so familiar with thread safety, so you may be right. Can you 
> explain a potential harmful scenario ?

A particularly plausible one? No.
Just some scenario? Yes.
Some application could implement a kind of certificate check and wants
to check if it is still valid. Due to some design idiocy it does that
via localtime(time) (yes, this part is very contrived. It is correct
that localtime should not be used by almost anything except maybe stuff
displayed to the user).
It does that in a separate thread to allow encoding at the same time.
Right after that localtime finishes, there is a timer interrupt and now
the libavformat thread runs, calling localtime to convert the MXF
timestamp (which refers to a date some years back).
Some time later the checking thread runs again and will check the
end-of-validity date against the years-old date of the video instead of
the current date, since the localtime call in libavformat overwrote the
buffer.

[...]
> I can understand the thread-local store, here people using the virtual 
> mechanism are really looking into problems by using localtime.

Actually, forget about that. Thread-local store will break down with
exactly the same problems and more when you pass the pointer returned
by localtime to a different thread, which is not forbidden
(although less likely).

> > Btw. I am against localtime because that means when transcoding (once
> > we support transferring this information) the result will depend on
> > the local time zone (even when both source and destination are MXF, due
> > to summer time, conversion localtime ->  time_t ->  localtime is not
> > lossless - I think).
> 
> For now, since my application reads the time "as is", Im against using 
> GMT time, unless stated in specs.

Local time has serious issues, and usually has values of 1 hour once a
year it can not represent and once a year that is ambiguous (the later one
meaning it is not really suitable for e.g. security cameras), and those differ
by country.
Using localtime is a huge hack, and IMO that should be in your
application if you want it.
It is easy to do this hack in your application, just do
timestamp = timegm(localtime(time)) instead of
timestamp = time
You will still have all the issues (including all the fun should you
ever use "cloud computing" where randomly a server might be located in a
different timezone), but at least other users will be able
to do things in a non-broken way without having to change their timezone
to UTC.
A somewhat nicer way might be to add a "struct tm" instead of just the
timestamp by the user, this would also allow to encode values before and
after the unix epoch which I think currently is not possible.

Greetings,
Reimar D?ffinger