[FFmpeg-devel] [PATCH] Adding support for OSX bundles to ffmpeg command line

[Måns Rullgård]

Chris Marrin <chris at marrin.com> writes:

> On Mar 20, 2009, at 2:37 PM, M?ns Rullg?rd wrote:
>
>> Chris Marrin <chris at marrin.com> writes:
>>
>>> On Mar 20, 2009, at 9:50 AM, M?ns Rullg?rd wrote:
>>>
>>>> Chris Marrin <chris at marrin.com> writes:
>>>>
>>>>> @@ -3915,7 +3930,16 @@
>>>>> {
>>>>>    int i;
>>>>>    int64_t ti;
>>>>> -
>>>>> +
>>>>> +    // get the executable path
>>>>> +    char* p = strrchr(argv[0], '/');
>>>>> +    if (p) {
>>>>> +        int length = p-argv[0];
>>>>> +        executable_path = av_malloc(length+1);
>>>>> +        strncpy(executable_path, argv[0], length);
>>>>> +        executable_path[length] = '\0';
>>>>> +    }
>>>>> +
>>>>
>>>> There are a million ways in which this can fail.
>>>
>>> I can find a few:
>>>
>>> 1) executable_path is not set to null if the if test fails (along  
>>> with
>>> the corresponding check for null where used).
>>> 2) The malloc can fail.
>>> 3) In the unimaginably unlikely case that argv[0] contained a string
>>> of greater than 2GB with a slash at the end, the length would wrap
>>> 4) The executable_path is leaked (not a failure, but I can fix it)
>>>
>>> I can fix all those. Others?
>>
>> 5) argv[0] doesn't contain the (expected) path to the executable.
>> There are a number of ways this can realistically happen.
>
> The outcome of which is that a) the string looks nothing like a path  
> and executable_path is empty or b) the string looks something like a  
> path and executable_path contains nonsense. In either case you would  
> not find the preset and fallback to existing behavior. I suppose  
> argv[0] could even be null, so a check for that is needed?
>
> Any more dangers?

It could point to a location containing presets, but the wrong ones.
It could use something other than / as directory separator.

Using argv[0] is generally wrong, period.

-- 
M?ns Rullg?rd
mans at mansr.com