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
