Comment # 3 on bug 1176742 from Matthias Gerstner

(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.