Bugzilla – Bug 1170161
AUDIT-FIND: enlightenment: enlightenment_system: _store_mount_verify() follows symlinks in /media/$user
Last modified: 2022-07-05 10:56:13 UTC
+++ This bug was initially created as a clone of Bug #1169238
This function rejects relative path components in the target mount path,
unlike its sibbling function described in b). However, it is unaware of
symlinks. Furthermore it makes sure that /media/$user and /media/$user/sub are
existing and are owned by the $uid:$gid of the unprivileged user.
- by placing a symlink in /media/$user/sub the setuid-root binary can be
tricked to create attacker owned directories in arbitrary locations.
This can quite likely lead to full root access by creating user owned
directories e.g. beneath /etc that are then used by other privileged
- if the attacker wins a race condition he can also cause the setuid-root
binary to pass ownership of arbitrary existing directories to himself. The
`_store_mount_verify()` functions performs a single `stat()` call on the
target mount path. Only if it exists and is not owned by the unprivileged
user, the execution is aborted. Therefore if the attacker places a suitable
symlink in the target path just after this `stat()` is performed by the
setuid-root binary, the following `_mkdir()` invocation will `mkdir()` and
`chown()` the path components nonetheless. This allows full root system
access by gaining ownership of e.g. /etc or /root.
To fix this I suggest not to pass ownership of /media/$user or of any
sub-directories to the unprivileged user. If /media/$user is user controlled
then the mount operation should be rejected.
Upstream Fix: https://phab.enlightenment.org/rE0c79c6317b92d9a5dd2548bb34625c6ed03018d7
enlightenment is not actually using this codepath yet.
The upstream fix should deal with the problem at hand. I still need to check
what happens with symlinks.
Now the code performs a chown to 0:0 for the /media and /media/$user
directory. It stills seems a bit forced, could potentially clash with other
mount managers. But there's a note that they think about using /run/media
As I said before this logic looks still a bit shaky. The `_mkdir()` function
still performs a `chown()`, where a `lchown()` would be more on the safe side.
Theoretically, if another mount manager creates user owned /media/$USER
directories, then the symlink attack would still basically work at least one
time. A user could then place /media/$USER/link -> /etc. enlightenment_system
mkdir /media; chown 0:0 /media
mkdir /media/$USER; chown 0:0 /media/$USER
mkdir /media/$USER/link; chown 0:0 /media/$USER/link
Since the chown is now towards 0:0 the attack isn't that useful any more.
But a mount over a symlink could still cause trouble.
Created attachment 838069 [details]
patch to harden _mkdir()
In attachment 838069 [details] you can find a suggested patch that would make me sleep
better regarding this. Can you approach upstream with this?
Sorry I didn't mean to close this bug just yet.
Could I please get an update about this? Can you add the patch?
Patch to remove the code is https://build.opensuse.org/package/view_file/X11:Enlightenment:Factory/enlightenment/feature-openSUSE-disable-system-storage.patch?expand=1 sorry it took so long
Thanks for addressing this. With the code removed the mkdir patch is not
necessary. Closing as fixed.