[Bug 1170162] New: AUDIT-FIND: enlightenment: enlightenment_system: _store_umount_verify(): does not protect against shell metacharacters and relative path components
http://bugzilla.suse.com/show_bug.cgi?id=1170162 Bug ID: 1170162 Summary: AUDIT-FIND: enlightenment: enlightenment_system: _store_umount_verify(): does not protect against shell metacharacters and relative path components Classification: openSUSE Product: openSUSE Tumbleweed Version: Current Hardware: Other OS: Other Status: NEW Severity: Normal Priority: P5 - None Component: Security Assignee: simonf.lees@suse.com Reporter: matthias.gerstner@suse.com QA Contact: qa-bugs@suse.de CC: security-team@suse.de Blocks: 1169238 Found By: --- Blocker: --- +++ This bug was initially created as a clone of Bug #1169238 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. -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1170162 Matthias Gerstner <matthias.gerstner@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |matthias.gerstner@suse.com -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1170162 http://bugzilla.suse.com/show_bug.cgi?id=1170162#c1 --- Comment #1 from Simon Lees <simonf.lees@suse.com> --- Upstream: https://phab.enlightenment.org/T8671 -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1170162 http://bugzilla.suse.com/show_bug.cgi?id=1170162#c2 --- Comment #2 from Simon Lees <simonf.lees@suse.com> --- Upstream Commit: https://phab.enlightenment.org/rE800ff4e24ff1a48dfa97f0cf6fe2d70c6768533b -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1170162 http://bugzilla.suse.com/show_bug.cgi?id=1170162#c3 Matthias Gerstner <matthias.gerstner@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |IN_PROGRESS --- Comment #3 from Matthias Gerstner <matthias.gerstner@suse.com> --- The upstream commit should fix the issue. I would have liked to see some common logic between the different commands instead of coding this complex query again. It's very hard to read... -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1170162 http://bugzilla.suse.com/show_bug.cgi?id=1170162#c4 --- Comment #4 from Matthias Gerstner <matthias.gerstner@suse.com> --- 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. Can you approach upstream with this? -- You are receiving this mail because: You are on the CC list for the bug.
http://bugzilla.suse.com/show_bug.cgi?id=1170162 http://bugzilla.suse.com/show_bug.cgi?id=1170162#c5 --- Comment #5 from Simon Lees <simonf.lees@suse.com> --- (In reply to Matthias Gerstner from comment #4)
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.
Can you approach upstream with this?
I have approached upstream with these, I suspect the reason for not using the umount2 system call is that the mount / unmount section of this helper binary is only intended to be used as a fallback for systems with no udisks2 support ie bsd's, I suspect the safest fix is to conditionally remove the support at compile time when udisks is detected. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1170162 https://bugzilla.suse.com/show_bug.cgi?id=1170162#c6 --- Comment #6 from Matthias Gerstner <matthias.gerstner@suse.com> --- Any news about the umount system call and upstream? -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1170162 https://bugzilla.suse.com/show_bug.cgi?id=1170162#c7 --- Comment #7 from Simon Lees <simonf.lees@suse.com> --- Following up here again sorry for the delay. The response from upstream was this is a fallback for systems that don't have udisks which is present on most Linux systems, so generally this code was designed to run on BSD systems which enlightenment does support. I don't believe most BSD systems include the umount2 syscall so it isn't really a solution. This code won't run on openSUSE so I can patch it out if it makes you more comfortable. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1170162 https://bugzilla.suse.com/show_bug.cgi?id=1170162#c8 --- Comment #8 from Matthias Gerstner <matthias.gerstner@suse.com> --- Since the code, if run, would be a realistic security issue, as outlined in comment 4, I would appreciate a patch that removes the code. If the fallback is not relevant for Linux then it should not hurt apart from the burden of carrying the patch. -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1170162 https://bugzilla.suse.com/show_bug.cgi?id=1170162#c9 --- Comment #9 from Simon Lees <simonf.lees@suse.com> --- Patch to remove the code is https://build.opensuse.org/package/view_file/X11:Enlightenment:Factory/enlig... sorry it took so long -- You are receiving this mail because: You are on the CC list for the bug.
https://bugzilla.suse.com/show_bug.cgi?id=1170162 https://bugzilla.suse.com/show_bug.cgi?id=1170162#c10 Matthias Gerstner <matthias.gerstner@suse.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|IN_PROGRESS |RESOLVED Resolution|--- |FIXED --- Comment #10 from Matthias Gerstner <matthias.gerstner@suse.com> --- okay, thanks for addressing this. closing this bug as fixed. -- You are receiving this mail because: You are on the CC list for the bug.
participants (2)
-
bugzilla_noreply＠novell.com -
bugzilla_noreply＠suse.com