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.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
