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

[Ramiro Polla]

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.