On Sun, Aug 18, 2019 at 10:47:26AM +0200, Tomas Härdin wrote: > sön 2019-08-18 klockan 02:35 +0200 skrev Tomas Härdin: > > lör 2019-08-17 klockan 17:33 +0200 skrev Michael Niedermayer: > > > On Sat, Aug 17, 2019 at 12:26:27AM +0200, Tomas Härdin wrote: > > > > fre 2019-08-16 klockan 14:57 +0200 skrev Tomas Härdin: > > > > > > > > I feel I should point out that being conservative here is at odds with > > > > the general "best effort" approach taken in this project. These toy > > > > codecs are useful as illustrative examples of this contradiction. I'm > > > > sure there are many more examples of files that can cause ffmpeg to do > > > > a lot more work than expected, for almost every codec. I know afl-fuzz > > > > is likely to find out that it can make the ZMBV decoder do a lot of > > > > work for a small input file, because I've seen it do that with gzip. > > > > > > > > The user base for cinepak is of course miniscule, so I doubt anyone's > > > > going to complain that their broken CVID files don't work any more. I > > > > certainly don't care since cinepakenc only puts out valid files. > > > > But > > > > again, for other formats we're just going to have to tell users to put > > > > ffmpeg inside a sandbox and stick a CPU limit on it. Even ffprobe is > > > > vulnerable to DoS-y things. > > > > > > yes > > > > > > the question ATM is just what to do here about this codec ? > > > apply the patch ? > > > change it ? > > > > Well for a start, the file is 65535 x 209 pixels, 3166 frames. I > > wouldn't call decoding that @ 263 fps particularly slow > > > > Second, it's not the decoder which is slow. If I comment out the > > "*got_frame = 1;" then the test also runs fast. I'm not sure what > > happens elsewhere with the decoded buffer, but I suspect there's a > > bunch of useless malloc()/memset()ing going on. Maybe the decoder is > > using ff_reget_buffer() or av_frame_ref() incorrectly, I'm not sure. > > I did some investigation, it is indeed ff_reget_buffer(). It copies the > frame data for some reason. The fix is simple in this case: just call > ff_get_buffer() once in cinepak_decode_init() and keep overwriting the > same frame. > > > As I said on IRC, this class of problems will exist for every codec. > > Cinepak is easy to decode, even at these resolutions. Just imagine what > > will happens when someone feeds in a 65535x209 av1 stream.. > > And related to this, ff_reget_buffer() is used for a lot of these > codecs which only overwrite pixels in the old frame. flicvideo, gifdec, > msrle, roqvideodec and others probably have the same flaw.
not calling any form of *get_buffer per frame breaks decoding into user supplied buffers. If you check the documentation of the get_buffer2 callback " This callback is called at the beginning of each frame to get data buffer(s) for it." That would not be possible if its just called once in init and yes i too wish there was a magic fix but i think most things that look like magic fixes have a fatal flaw. But maybe iam missing something in fact i hope that iam missing something and that there is a magic fix PS: if you think of changing the API, i dont think its the API. I mean every user application will read the frames it receives, so even if inside the decoder we just return the same frame with 2 pixels different the user doesnt know this and has to read the whole frame. The problem is moved around but its still there. Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB During times of universal deceit, telling the truth becomes a revolutionary act. -- George Orwell
signature.asc
Description: PGP signature
_______________________________________________ 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".
