On 5/11/18, Derek Buitenhuis <derek.buitenh...@gmail.com> wrote: >> please correct me if iam wrong, theres quite a bit iam guessing here >> IIUC the problem is that in your usecase >> 1. ffmpeg has access to sensitive files >> 2. one of these files can be opened by an attacker with ffmpeg >> 2b. This involves the file being probed as a supported format > > It is "probed" as file extension mostly. .txt is one of these > extensions, which 99.999999999999999% > of the time, is not a multimedia file, for example. > >> 3. The file is then demuxed, decoded, encoded, muxed >> 4. the result is given back to the attacker >> >> This patchset removes one of the demuxers involved in the attack > > Not removed. Disabled. As discussed in other replies, making it manual > also seems > fine to me. Rendering arbitrary text files as images (as in, rendering > the text using > a built in font, to an image with that text in it) isn't exactly a > great and secure default > behavior, especially for the world's most commont text file extension... > >> The first problem of this patchset is that it does not provide any >> evidence of how the other demuxers probe functions can trigger on >> random text files. > > ffmpeg -i something.txt a.png > > But really, it is not necssarily an attack on its own (it could be), > but it makes any > other attack vastly easier to exploit. For example, that HLS stuff > avformat had fixed > last year could have pointed at various .txt files. I don't really > understand why the > concept of "rendering arbitrary text files as images" is not obviously bad? > >> for example idf_probe() requires, the first 12 bytes of the file to >> match exactly and some of these are not text. So a attack which depends >> on the probing of sensitive text data to succeed should not work with it > > it is not specifci to probing. It's choosing e.g. the ansi decoder based off > the > .txt file extension, for example. Or the bintext decoder based off the > .bin extension. > >> The second problem i see is that the patch disables the demuxers, while >> disabling the probe functions affected should have the same effect >> security wise but is a smaller step in respect to disabling. > > As noted in my reply to Marton and Gyan, I'm fine with this approach, > as the implications > are the same. > >> The third problem i see is that really once you read sensitive data >> and pass something back to the user you already lost. > > [...] > >> A text demuxer and decoder makes it easier and it makes it much easier >> to demonstrate the attack in a flashy way. But having the probe code >> access the sensitive data even if none succeeds already makes quite >> a bit of internal state (caches, branch prediction, deallocated memory, >> ...) >> be contaminated with sensitive information. Its hard to ensure that >> none of this can be recovered by the attacker. > > Ah the classic "well it can technically be beaten, you may as well make it > as easy as possible" argument that people use to argue against things > like ASLR... :| > > The point is to minimize the risk where possible, with good defaults for > *all* > users. It's not a great thing to just point at users and go "well you > should have > done a better job containerizing/sandboxing/etc." IMO. > >> I think first ffmpeg should not be able to access sensitive data. >> Then none of our probe functions should succeed on the average >> sensitive text file. If one does, we should look into making it not >> detect that. > > The succeed because libavformat loves probing based on file extenions. > >> Also it may seem counter-intuitive but adding a probe function which >> is designed to succeed on sensitive text files may be more reliable >> to stop their detection than to disable probe functions. > > This is just adding more heuristics to probing, though > >> The probe system is basically doing differential probing. That is >> the one of 2 that has the higher score wins. >> So it may be more effective to add a function that that returns a >> high score intentionally for a null demuxer than to try to stop >> every function that returns more than 0 > > Wouldn't this break a lot of currently "working" detection of > bad/broken files that > users seem to rely on? (I don't think they should rely on it, but > that's just my opinion.) > >> if such probe code is added, it also could maybe warn the admin about a >> potential misconfiguration that allows probing to reach to sensitive >> data. > > [...]
If you think you are fixing security issue you are very wrong. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
