Bug#661627: Avoid /tmp ?

[vladz <vladz@devzero.fr>]

Julien, thank you for putting me back in CC. ;)

On Thu, Mar 01, 2012 at 09:48:47PM +0100, Julien Cristau wrote:
> On Thu, Mar  1, 2012 at 12:39:41 -0800, Tim wrote:
> > > > Note that the "chown root:root $SOCKET_DIR" also seems redundant to me
> > > > (if we didn't already own it, we would have bigger problems, right?).
> > > > 
> > > I guess it protects against some user doing mkdir /tmp/.X11-unix before
> > > this runs (which probably means before the package is installed, so it's
> > > not like this is a very likely race) and then owning the directory.
> > 
> > Oh, right, duh.  Well, the dir is created every time the box boots,
> > since /tmp is cleared, so it is needed for sure.

I think the obsolete chown command should be removed (as said Tim), and
also the chmod should by replaced by a single atomic operation (using 
"mkdir -m").  Those two things will avoid usages of dangerous commands
and then, reduce TOCTTOU risks.

So if chown is removed, another check has to be introduced.  It is to
check if the directory isn't owned by root.  Because a user could try to
own X sockets by creating the directory himself.

Well, here is how I would see the script (with a lot of comments to try
making things clear):

  # We move $SOCKET_DIR if:
  #   - it exists but is not a dir
  #   - is whatever but not owned by root
  #   - is a symlink
  if { [ -e $SOCKET_DIR ] && [ ! -d $SOCKET_DIR ]; } ||
     { [ -e $SOCKET_DIR ] && [ ! -O $SOCKET_DIR ]; } ||
       [ -h $SOCKET_DIR ]; then

    mv $SOCKET_DIR $SOCKET_DIR.$$
  fi


  # At this point, we could find:
  #  - Correct directory (ie. $SOCKET_DIR owned by root)
  #  - a symlink or whatever (raced by a malicious user)
  #
  # So before creating it, we need to check if:
  #  - does not exist or is not owned by root
  if [ ! -O $SOCKET_DIR ];
    # symlink, malicious files will give a failure here
    mkdir -m 1777 $SOCKET_DIR
  fi

What do you think about this?

To finish, I find the ways to create those two directories ($SOCKET_DIR
and $ICE_DIR) quite redundant.  A function called create_dir() could 
contain the code above and be launched from both set_up_socket_dir() and
set_up_ice_dir()?