Page MenuHomePhabricator

enlightenment_system: _store_umount_verify(): does not protect against shell metacharacters and relative path components
Closed, ResolvedPublic


This function tries to make sure that the user can only unmount his own mounts
below /media/$user. It also rejects backslashes in the path. However it does
not reject relative path components or shell characters.

  • this allows a regular user to unmount arbitrary file systems by passing paths like "/media/$user/../../tmp.
  • since the unmount is performed by calling the umount utility via "/bin/sh", shell metacharacters will be interpreted. Passing a path like '/media/testuser/$(date)' will cause the setuid-root program to execute the date program as root. This leads to full code execution as root. The only requirement is that a directory of the same name exists. Spaces are also allowed in the path, therefore even complex commands can be executed as root.

I recommend to reject relative path components and shell metacharacters in
this function to fix the issue.

simotek created this task.Apr 22 2020, 3:50 AM
simotek triaged this task as High priority.

The umount has another possible attack surface. Even if the program only
allows removable devices to be mounted ... I could insert a removable device
with a UNIX file system on it that contains symlinks.

Then I could point the umount command to /media/$user/somemount/somelink and
the link target would be unmounted. The problem is that the umount program
is used for unmounting, which follows symlinks.

It would be better to use the umount2 system call and pass "UMOUNT_NOFOLLOW"
to avoid symlinks being followed. Another approach could be to forbid slashes
after /media/$user/somemount and making sure that /media and /media/$user
aren't user controlled.

Also unmount wasn't disabled in the same way as mount.

indeed the umount is not checked to be precisely a 3 element path. as the whole storage is disabled it's not an accessible code-path right now so not a security issue that can be exploited right now but once it's enabled - yes. i need to just ensure it's a limited path (/media/username/single_dir_only and nothing beyond that - as they are all owned by root, there is nothing a user can do to abuse this.