[MPlayer-cvslog] r32731 - in trunk: path.c path.h

[Reimar Döffinger]

On 26 dec 2010, at 12:04, KO Myung-Hun <komh78 at gmail.com> wrote:
> Reimar Döffinger wrote:
>> On 26 dec 2010, at 09:22, KO Myung-Hun <komh78 at gmail.com> wrote:
>>> Clément Bœsch wrote:
>>>> On Sun, Dec 26, 2010 at 02:12:19AM +0900, KO Myung-Hun wrote:
>>>>> Hi/2.
>>>>> 
>>>>> cboesch wrote:
>>>>>> Author: cboesch
>>>>>> Date: Sat Dec 25 00:09:08 2010
>>>>>> New Revision: 32731
>>>>>> 
>>>>>> Log:
>>>>>> Add mp_path_is_absolute function.
>>>>>> 
>>>>>> Modified:
>>>>>>  trunk/path.c
>>>>>>  trunk/path.h
>>>>>> 
>>>>>> Modified: trunk/path.c
>>>>>> ==============================================================================
>>>>>> --- trunk/path.c    Sat Dec 25 00:05:49 2010    (r32730)
>>>>>> +++ trunk/path.c    Sat Dec 25 00:09:08 2010    (r32731)
>>>>>> @@ -233,3 +233,15 @@ char *mp_dirname(const char *path)
>>>>>>    dirname[len] = '\0';
>>>>>>    return dirname;
>>>>>> }
>>>>>> +
>>>>>> +/**
>>>>>> + * @brief Indicates weither the path is absolute or not.
>>>>>> + */
>>>>>> +int mp_path_is_absolute(const char *path)
>>>>>> +{
>>>>>> +#if HAVE_DOS_PATHS
>>>>>> +    return path[0] && path[1] == ':';
>>>>>> +#else
>>>>>> +    return path[0] == '/';
>>>>>> +#endif
>>>>>> +}
>>>>>> 
>>>>> 
>>>>> You should consider the case that path[0] is '/' or '\\' as well in
>>>>> HAVE_DOS_PATHS
>>>>> 
>>>> 
>>>> Mmmh ok, I'll fix that. But I wonder how opendir() will behave in those
>>>> cases (I don't have any windows machine to test).
>>>> 
>>> 
>>> It uses a current drive.
>> 
>> Which is what makes this complicated, /something and \something are _not_ absolute paths on windows, they do depend on the current directory.
> 
> If so, are you saying that it will be a x:\current_dir\something when
> creating \something on Windows ?
> 
> It will be x:\something.

Yes, but x depends on the current path.

> 
>> Neither is c: btw., only c:/ and c:\ and \\something are absolute paths.
> 
> But PathIsRelative() return 0 in case of \something, c: and c:\.
> 
> I think, the path which can be appended to 'x:\dir\' is relative,
> otherwise absolute.

Security-wise this is a very, very dangerous way to define it, if someone can trick you into changing the current directory (which btw. the file open dialog will do), \something will suddenly be a completely different location than it was just before.
Maybe it makes most sense in the end, but not without a _huge_ warning note in the documentation and reviewing what this means for how the code behaves.
For subtitles this is less critical, but for example if it was used for config files getting this wrong might mean reading them from a public network drive, which would be trivial to exploit.