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

[KO Myung-Hun]

Reimar Döffinger wrote:
> 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.

Yes, but you should consider the case that people append a relative path
to a current path.

\something and c: generate a incorrect path.

I agree absolutely with you that these codes should be reviewed
thoroughly for a security.

And it would be better to provide a function to composite a absolute
path from a relative path.

This can reduce the errors on OS using DOSish path.

-- 
KO Myung-Hun

Using Mozilla SeaMonkey 2.0.11
Under OS/2 Warp 4 for Korean with FixPak #15
On Intel Core2Duo T5500 1.66 GHz with 2 GB RAM

Korean OS/2 User Community : http://www.ecomstation.co.kr