From: Jeff King <peff@peff.net>
Date: Thu, 12 Nov 2009 08:35:59 GMT
Subject: Re: [PATCH] gitk: Add ability to define an alternate temporary directory
Message-ID: <20091112083559.GB17552@coredump.intra.peff.net>
In-Reply-To: <20091111164451.GA45475@gmail.com>

On Wed, Nov 11, 2009 at 08:44:53AM -0800, David Aguilar wrote:

> gitk writes file@commit and file@commit^ to temporary files
> and diffs them using an external diff tool.
> 
> Shall I reroll with s/GITK_TMPDIR/TMPDIR/ ?

gitk seems to use a very predictable temp filename (".gitk-tmp.$PID").
Have you checked that you are not introducing any security holes by
creating that predictable filename in a publicly writable tmpdir?

It looks like it always tries to "mkdir" the file. Does tcl's mkdir
function barf on an existing directory? If so, then I think we may be
safe from the naive:

  tmp=.gitk-tmp.`pidof_other_users_gitk`
  mkdir $tmp
  ln -s /file/for/other/user/to/destroy /tmp/1

attack. And I think we are not susceptible to races because we fail if
the mkdir fails (instead of doing something stupid like stat followed
by mkdir).

But it has been a long time since I thought about /tmp security, so I
may be forgetting something.

-Peff