https://bugzilla.suse.com/show_bug.cgi?id=1176742 https://bugzilla.suse.com/show_bug.cgi?id=1176742#c3 --- Comment #3 from Matthias Gerstner <matthias.gerstner@suse.com> --- (In reply to fvogt@suse.com from comment #1)

I had a quick look at the helper and while it prevents arbitrary arguments to be passed (the absolute path has to be below /dev), this can probably be circumvented by sending a file descriptor over dbus and passing /dev/fd/X (should be predictable) as argument.

Yes this would be possible, but it requires the attacker to open the file in advance (i.e. he needs to have certain access rights anyway). It could be combined with a symlink attack in the path that /dev/fd/X is pointing to. Furthermore there is a TOCTOU race condition regarding the call to `info.absoluteFilePath()` and the time the path is actually accessed by smartctl. Since /dev/shm is a world writable sticky bit directory an attacker can: - first place a regular file in /dev/shm/myfile - call the org.kde.kded.smart.smartctl action for /dev/shm/myfile - try to win the race condition by replacing /dev/shm/myfile with a symlink at the right time - on success smartctl will dereference the symlink and arbitrary paths will become possible. - kernel symlink protection will prevent the worst Mostly this issue could be used for file existence tests. Since smartctl only operates on block devices there shouldn't be much further damage involved. Since we are seeing these types of weak file path checks again and again I still like to see improved code. We need to stop spreading such bad examples in OSS code. What can be done is something along the following lines: - use realpath() to resolve all path components of the input path - require that the resolved path starts with /dev - split up the path into individual path components - open /dev with the O_PATH|O_NOFOLLOW flags and start descending down the resolved path components using openat(), O_PATH|O_NOFOLLOW. for each path component: - perform an fstat() and inspect the owner, file type and mode. - if the file type is other than block device or directory then an error needs to be returned. - if the file is owned by a non-root user then an error needs to be returned. - if the file is world-writable (sticky-bit directories) then an error needs to be returned. - the final path component needs to be of the block device file type - smartctl needs to be pointed to the resolved path. No path components under attacker control should be involved. -- You are receiving this mail because: You are on the CC list for the bug.