[pulseaudio-discuss] [PATCH v2] core-util: Fail if XDG_RUNTIME_DIR belongs to someone else

[Alexander E. Patrakov]

09.09.2014 12:52, David Henningsson wrote:
>
>
> On 2014-09-09 07:25, Alexander E. Patrakov wrote:
>> 09.09.2014 11:10, David Henningsson wrote:
>>>
>>>
>>> On 2014-09-08 17:24, Rémi Denis-Courmont wrote:
>>>>> diff --git a/src/pulsecore/core-util.c b/src/pulsecore/core-util.c
>>>>> index d7a95d6..6bb6317 100644
>>>>> --- a/src/pulsecore/core-util.c
>>>>> +++ b/src/pulsecore/core-util.c
>>>>> @@ -1816,6 +1816,14 @@ char *pa_get_runtime_dir(void) {
>>>>>      /* Use the XDG standard for the runtime directory. */
>>>>>      d = getenv("XDG_RUNTIME_DIR");
>>>>>      if (d) {
>>>>> +        struct stat st;
>>>>> +        if (stat(d, &st) == 0 && st.st_uid != getuid()) {
>>>>
>>>> This looks like a case of ToCToU to me.
>>>>
>>>> In principles, you should probably use open() then fstat(), and then
>>>> openat to create or access files within the directory.
>>>
>>> Thanks for the review. You're right. In this case however, we want to
>>> prevent root from doing a chown on XDG_RUNTIME_DIR by mistake.
>>
>> Then the code is too convoluted to express the simple idea that chown
>> must be done only when running in system mode.
>
> Hmm, maybe we should instead convolute pa_make_secure_dir to have a
> "verify, and fail instead of chown" option.

This looks like a good idea. And if you look in the history, you'll find 
that it was the only option in the distant past.

-- 
Alexander E. Patrakov