On Tue, Dec 05, 2017 at 11:21:00AM +0100, Salvatore Mesoraca wrote: > 2017-11-30 17:30 GMT+01:00 Solar Designer <solar@xxxxxxxxxxxx>: > > $ strace flock /tmp/lockfile -c cat > > [...] > > open("/tmp/lockfile", O_RDONLY|O_CREAT|O_NOCTTY, 0666) = 3 > > flock(3, LOCK_EX) = 0 > > > > This use of flock(1) would be a worse vulnerability, not limited to DoS > > against the script itself but also allowing for privilege escalation > > unless (sym)link restrictions are enabled in the kernel. Adding O_EXCL > > would help (reduce the impact back to DoS against itself), and would > > require that the retry logic (like what is seen in the lock directory > > example above) be present. > > > That behavior can be certainly avoided, but of course it isn't a > > > security problem per se. > > > > I think it is a security problem per se, except in the "subtle case" > > above, and it's great that our proposed policy would catch it. > > I agree on the DoS, though, at first, I didn't consider it a "bug" because > there isn't any open mode that can prevent the DoS in this case. > If you want to avoid it, you must avoid other-users-writable directories > at all. So, It think that, if you are using a sticky directory, it's > intended behavior to let someone else "lock" you. Right. There's a worse DoS I had overlooked, though: flock(1) can also be made to create and/or lock another file (maybe in another directory). Perhaps adding O_NOFOLLOW (alongside flock(1)'s existing use of O_NOCTTY) would be a good idea, even though uses in a directory writable by someone else are inherently risky anyway. > But maybe many flock(1) users are not aware of the issue and so, sometimes, > it can be unintended. > I didn't consider privilege escalation as an issue because I always > looked at flock(1) under the assumption that the lockfile is never actually > read or modified in any way and so it shouldn't make too much difference if > it's an already existing regular file or a symlink etc. > Am I missing something? You made a good point, but yes: O_CREAT will follow a dangling symlink and there are cases where creating an empty file of an attacker-chosen pathname may allow for privilege escalation. For example, crontab(1) man page on RHEL7 says regarding /etc/cron.allow and /etc/cron.deny: "If neither of these files exists, only the super user is allowed to use cron." In that configuration, simply creating empty /etc/cron.deny grants access to crontab(1) to all users. As user: $ crontab -l You (solar) are not allowed to use this program (crontab) See crontab(1) for more information $ ln -s /etc/cron.deny /tmp/lockfile As root: # sysctl -w fs.protected_symlinks=0 fs.protected_symlinks = 0 # flock /tmp/lockfile -c echo As user: $ crontab -l no crontab for solar There may also be side-effects on open of device files (the best known example is rewinding a tape), and we won't avoid those by retrying without O_CREAT|O_EXCL. O_NOFOLLOW will help against symlinks to device files, but not against hard links (if on same device). The kernel's symlink and hardlink protections help, but in this sub-thread we were discussing detecting userspace software issues without waiting for an attack. Things like this fit David Laight's point well: programs trying to make risky (mis)uses less risky sometimes also avoid being detected by our proposed policy. That's life. Alexander