[systemd-devel] "CVE-2013-4392: TOCTOU race condition when updating file permissions and SELinux security contexts" still an issue

[Lennart Poettering]

On Sa, 24.03.18 22:54, Michael Biebl (mbiebl at gmail.com) wrote:

> Hi,
> 
> the Debian systemd package has an open bug report
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=725357
> about
> CVE-2013-4392: TOCTOU race condition when updating file permissions
> and SELinux security contexts
> 
> This references https://bugzilla.redhat.com/show_bug.cgi?id=859060
> which never was properly closed afaics. So I wonder if this issue is
> still relevant or was this fixed in another way?

Well, I figure the original intention was to fix this in libselinux or
the kernel, but to my knowledge this never happened, most selinux code
is still vulnerable to this class of attacks as ever. However, in the
meantime O_PATH was added, which allows us to work around this issue
in userspace. Moreover, the fs.protected_hardlinks sysctl was added
which makes part of the issue go away.

Note that a similar set of problems actually applies to the SMACK
relabelling code, however it's much less problematic there, as the
only policy they apply by default is on file nodes below /dev, which
is a pretty restricted namespace (that said, world-writable /dev/shm
is located too there, so maybe this doesn't count too much).

I have now prepped this, which tries to do something about this for
both SELinux and SMACK, focussing on our own code only:

https://github.com/systemd/systemd/pull/8583

Which fixes one half of the issue, by moving things over to O_PATH: it
allows us to pin the file object while determining the label to use
and applying it, thus removing a TTOCTTOU race.

To fully fix the issues discussed in the original bug report, a second
part is missing though, that adds checks for .st_nlink > 1 á la
hardlink_vulnerable() in tmpfiles.c. Given that this is a non-issue on
reasonable systems (i.e. on all systems with fs.protected_hardlinks
set), I don't care too much about this though. But I am happy to
review a patch that makes the hardlink checking code from tmpfiles.c
generic and hooks it up with selinux relabelling too, if anyone cares.

#8583 also fixes the other issue you just reported: #8566.

Anyway, I wrote this PR without doing any thorough testing, since I
neither run SMACK nor SELinux. If you could test this and report back
this would be greatly appreciated.

Lennart

-- 
Lennart Poettering, Red Hat