[FFmpeg-devel] [PATCH] Add -fpre option.

[Ramiro Polla]

On Mon, Nov 2, 2009 at 9:45 PM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
> On Mon, Nov 2, 2009 at 8:01 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> On Mon, Nov 02, 2009 at 12:00:56PM -0200, Ramiro Polla wrote:
>>> On Mon, Nov 2, 2009 at 9:39 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>>> > On Mon, Nov 02, 2009 at 12:30:53PM +0100, Michael Niedermayer wrote:
>>> >> On Sun, Nov 01, 2009 at 10:53:37PM -0200, Ramiro Polla wrote:
>>> >> > msg7668 in issue659 spots another case where the preset files won't
>>> >> > open a valid path. Also "../" doesn't work for example. Attached patch
>>> >> > makes it the user's job to pass -fpre to open a preset file and FFmpeg
>>> >> > will just pass it on to open().
>>> >>
>>> >> your hack makes it impossible to specify different preset files
>>> >> for audio and video, that worked before ...
>>> >> also it complicates the interface
>>>
>>> 'a', 'v', 's' are only used to write the codec name to the filename,
>>> and when specifying 'f' that seemed unnecessary.
>>>
>>> >> is there a reason why open() is not always tried if all else fails?
>>>
>>> patch attached opt_preset_1.diff
>>>
>>
>>> Although I prefer the other way around (try open() and if that fails
>>> snprintf() the filename)
>>
>> that would be possibly exploitable
>> 1. user downloads archive
>> 2. user extracts files to new directory
>> 3. user enters directors
>> 4. user encodes a file from there with some common preset
>> 5. sadly there is a file matching that common preset in that archive
>> ? this gives an attacker full control over the command line parameters and
>> ? the input file, definitly easier for her than just the input file
>>
>>
>>>
>>> > or alternatively check for some prefix char and strip that from the
>>> > name to detect filenames or -fpre / fapre (audio) / fvpre (video) ...
>>>
>>> patch attached opt_preset_2.diff but that makes no difference at the
>>> moment from my first patch. This way I can do:
>>> ffmpeg -i input -vcodec libx264 -fapre some_libx264_preset.ffpreset output
>>>
>>> Is there any reason we separate [avs]pre besides to get the codec name
>>> to snprintf() into the filename? opt_preset() also allows specifying
>>> [avs]codec so a preset file could really change any codec and
>>> parameter...
>>
>> hmmmmmm, i thought there was some other use, but it seems not
>> well, if its really useless then fpre alone should be enough of course
>> and iam of course ok with that patch
>
> First patch on thread applied.

Baptiste suggested on irc that the option description was kind of confusing:
"set options to the indicated preset file"
and it kind of is really...
I suggest
"set options from preset file"
or
"read options from indicated preset file"

documentation maintainer, nitpickers, please comment (and/or just
commit something better)...

Ramiro Polla