2

I'm writing a daemon that monitors something in the OS and flips execution permissions on a file in /run/ back and forth. The file has static contents and the file name is hardcoded in the daemon. I made the daemon run as root to make it able to write to /run/.

The daemon is supposed to be used on a large number of desktop Linux machines, so I want to make sure it's not exploitable. I'm not particularly concerned by DDoS attacks though because the daemon is not mission-critical. However, all the ways to make a file executable or not executable seem to have a security flaw.

Way #1: chmod() back and forth

Create the file and chmod() it back and forth. I'm not fond with the idea because if an attacker manages to replace the file with a symlink to something else, an arbitrary file or folder will be chmod'ed into readability and possibly executability. The attacker would need root permissions for that, though.

Way #2: remove the file and atomically recreate it with open()

Delete the the file first and then atomically create a new one with the correct permissions using open(path, O_CREAT|O_WRONLY|O_TRUNC|O_EXCL, permissions), then write contents to the stream and close it. I like this one more because the only thing that can be exploited is removing a file with a hardcoded name by substituting the target directory with a symlink to some other directory, and if you can overwrite root dirs you should not need tricking daemons into removing a file with a hardcoded filename anyway.
What happens if the file gets deleted while the daemon is writing to stream in this case?
Is using g_remove() okay or shall I use unlink() only and abort if it fails?

Am I missing something or I'm just being paranoid and way #2 is secure enough?

Shnatsel
  • 2,802
  • 2
  • 16
  • 15
  • 2
    If an attacker needs `root` in order to do the attack in #1, then you have other things to worry about. If the attacker gets `root` assume all hope is lost on that system, because at that point they could install a rootkit or anything else...stop the daemon...etc. You best protection here is to just use SELinux and write a proper policy for what you want to do with context transitions. – sparticvs Nov 21 '12 at 01:05
  • Assuming the system already runs something like AppArmor, SELinux or TOMOYO, can my daemon be the cause of privilege escalation? It seems that way #1 allows converting the privilege to create symlinks as root into privilege to chmod things as root, doesn't it? – Shnatsel Nov 21 '12 at 01:11
  • 3
    Why are you flipping exec bits? – Jeff Ferland Nov 21 '12 at 01:27

2 Answers2

6

Your method #1 can be adapted to protect against symlink attacks:

  1. lstat() the file path you want to chmod. Save the values of st_dev and st_ino in the struct stat that is returned. These two numbers uniquely identify the file on your system.
  2. open() the file with O_RDONLY.
  3. fstat() the open file descriptor. Check to make sure that st_dev and st_ino are the same as the ones you saved earlier. If they're different, you've just followed a symlink and should bail out.
  4. fchmod() the file descriptor to set the execute bit. fchmod() is like chmod() except it operates on an open file descriptor.

If the attacker can't modify any ancestor directories of the directory containing this file, this is all you need to do. If the attacker can modify ancestor directories, then it's possible you followed a symlink on an earlier component of the path when you called lstat. Thus it gets a bit more complicated:

  1. lstat() the first component of the path which might be a symlink. Save st_dev and st_ino.
  2. chdir() into the next component of the path using a relative path.
  3. stat() the current directory (.) and make sure the st_dev and st_ino are the same.
  4. Repeat until you reach the directory containing the file.

When you've finally reached the directory containing the file, perform the original steps above, but make sure you call lstat() and open() on a relative path.

AGWA
  • 490
  • 4
  • 5
  • 1
    See the O_NOFOLLOW flag to `open(2)` as well (on FreeBSD and Linux at least) to fail if the file is the symlink and avoid having to check for race conditions. – Stéphane Chazelas Nov 21 '12 at 21:16
  • 1
    "fstat() the first component of the path which might be a symlink" - I assume you mean "stat()" here. That aside, thanks a lot, great answer! – Shnatsel Dec 29 '12 at 12:58
  • 1
    @shnatsel, actually it should be lstat(). Answer corrected. Thanks for pointing that out. – AGWA Jan 13 '13 at 07:02
1

Be aware: setting an executable as non-executable doesn't prevent it from running if it is readable (not to mention the concept of copying the file).

$ cp /bin/ls ~/bin/ls
$ chmod a-x ~/bin/ls
$ /lib/x86_64-linux-gnu/ld-2.15.so ~/bin/ls
#Yup, that worked.

... though you can prevent that if the file isn't readable, which doesn't stop execution.

$ chmod a+x ~/bin/ls 
$ chmod a-r ~/bin/ls
$ ~/bin/ls
#Worked
$ /lib/x86_64-linux-gnu/ld-2.15.so ~/bin/ls 
/home/me/bin/ls: error while loading shared libraries: /home/me/bin/ls: cannot open shared object file: Permission denied

It could prevent a program from running as suid, but then the answer to that probably doesn't involve setting the execute bit.

Jeff Ferland
  • 38,090
  • 9
  • 93
  • 171
  • In fact I'm merely changing it for executability checks in .desktop file handlers. I know the actual executability doesn't depend on that bit. The problem with the chmod()'s is that they set other properties such as readability in addition to flipping the executability bits. But thanks for the tip anyway! – Shnatsel Nov 21 '12 at 01:38
  • @Shnatsel What is the bigger goal you're trying to solve by flipping permissions bits? – Jeff Ferland Nov 21 '12 at 01:40
  • I'm trying to signal .desktop file visibility via TryExec= fields in them, http://standards.freedesktop.org/desktop-entry-spec/latest/ar01s05.html I've tried a bunch of other solutions but this seems to be the simplest even for more complex cases like checking several conditions, etc – Shnatsel Nov 21 '12 at 01:42
  • 3
    @Shnatsel If you'd like an answer, you really need to stop being vague and show the actual, concrete example of what you're doing. Indirection is wasting the time of people trying to help you. – Jeff Ferland Nov 21 '12 at 03:11