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.
I'm trying to think of a possible ways this could be exploited, but I
can't think of any. Either we will just fail later when trying to chown
XDG_RUNTIME_DIR, or we will succeed, and if we succeed that must be
because someone just chowned the directory away from us, and we're root
so we can take it back. And then that someone must be root, and we can't
protect us from root doing crazy stuff anyway.
Does that make sense? Security is always tricky.
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
