Re: [PATCH] gitk: Add ability to define an alternate temporary directory

From: Jeff King <peff@peff.net>
Date: Nov 12, 2009, 09:56 UTC
Message-ID: <20091112095620.GA7847@coredump.intra.peff.net>
In-Reply-To: <20091112093611.GA47285@gmail.com>

On Thu, Nov 12, 2009 at 01:36:13AM -0800, David Aguilar wrote:

Show 5 quoted lines

> When I have more time I can switch gitk over to git-difftool
> which I know is /tmp safe.  I only dabble in tcl but the
> docs say that mkdir does not error out when given
> directories that already exist.  It does error out when
> given a file.

OK, then I think we would be vulnerable, as I can make a .gitk-tmp.$PID directory owned by me that your gitk will happily use.

> The /tmp trick would require them knowing the SHA-1 that
> we're diffing and symlinking the names to paths they want
> us to destroy.  It seems paranoid to worry about it ;)

But the SHA-1 is not hard to guess[1], as you have a finite, easily-enumerable list of them in your repository. :) One thing that does make it harder is that gitk actually checks to see if a file is already there before creating it (presumably not for security, but for efficiency). Which means I can't just pre-seed a trap and wait for you to run gitk; I have to actually race you and create the file between your "file exists" check and the eventual "git show $filename >$output" which will hose it.

Probably I can win that race given a sufficient number of attempts, but attempts are made at a human pace. So in practice it's probably pretty hard to exploit. Still, I'd rather see it done properly on principle. Then we _know_ we're not missing some trick, and there's no chance of a later code change increasing an attacker's probability of success.

-Peff

[1] I was also going to suggest a social-engineering attack, like "hey, I screwed up my repository. Can you take a look?" Then you don't need to guess the SHA-1, as you convince the victim to look at a specific one. But that attack is already much, much worse: we respect items in .git/config regardless of whether it is owned by the running user. So it is not actually safe to "cd ~other_user/project && git diff".