Bug#661627: Avoid /tmp ?

[Julien Cristau <jcristau@debian.org>]

On Fri, Mar  2, 2012 at 12:44:23 +0100, vladz wrote:

> 
> 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.
> 
I'm not convinced the chown can be removed.  And 'mkdir -m 1777 foo' is
not any more atomic than 'mkdir foo && chmod 1777 foo'.  The problem is
that I'd still like to be able to chown and chmod /tmp/.X11-unix if it
already exists as a directory when the script runs.  I can do that in C
with something like this:

  ret = mkdir("/tmp/.X11-unix", 0700);
  if (ret == 0 || errno == EEXIST) {
    fd = open("/tmp/.X11-unix", O_RDONLY | O_NOFOLLOW);
    if (fd < 0)
      fail();
    fstat(fd, &st);
    if (!S_ISDIR(st.st_mode))
      fail();
    if (fchown(fd, 0, 0)) fail();
    if (fchmod(fd, 01777)) fail();
  } else {
    fail();
  }

but so far I haven't seen a way to do that in shell, because chmod(1)
doesn't have a --no-dereference option, and even if it did it doesn't
look like I could safely detect whether to exit with failure or success.

hmm, how about this:

mkdir -p /tmp/.X11-unix
chown -h root:root /tmp/.X11-unix
stat=$(LC_ALL=C stat -c '%u %g %F' /tmp/.X11-unix)
if [ "$stat" != '0 0 directory' ]; then
  exit 1
fi
chmod 1777 /tmp/.X11-unix

?

> 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()?
> 
Agreed.  Or drop those two functions and call set_up_dir "$SOCKET_DIR &&
set_up_dir $ICE_DIR" directly.

Thanks a lot to you both for the help so far...

Cheers,
Julien