improve temp dir security

http://bugs.debian.org/807341

* Fix insecure temporary permissions when git-annex repair is used in
  in a corrupted git repository.

  Other calls to withTmpDir didn't leak any potentially private data,
  but repair clones the git repository to a temp directory which is made
  using the user's umask. Thus, it might expose a git repo that is
  otherwise locked down.

* Fix potential denial of service attack when creating temp dirs.

  Since withTmpDir used easily predictable temporary directory names,
  an attacker could create foo.0, foo.1, etc and as long as it managed to
  keep ahead of it, could prevent it from ever returning.

  I'd rate this as a low utility DOS attack. Most attackers in a position
  to do this could just fill up the disk /tmp is on to prevent anything
  from writing temp files. And few parts of git-annex use withTmpDir
  anyway, so DOS potential is quite low.

Examined all callers of withTmpDir and satisfied myself that
switching to mkdtmp and so getting a mode 700 temp dir wouldn't break any
of them.

Note that withTmpDirIn continues to not force temp dir to 700.
But it's only used for temp directories inside .git/annex/wherever/
so that is not a problem.

Also re-audited all other uses of temp files and dirs in git-annex.

2 files changed, 32 insertions, 14 deletions

diff --git a/Utility/Tmp.hs b/Utility/Tmp.hs
index 7e4db1101..7610f6cc8 100644
--- a/Utility/Tmp.hs
+++ b/Utility/Tmp.hs
@@ -15,6 +15,9 @@ import System.Directory
 import Control.Monad.IfElse
 import System.FilePath
 import Control.Monad.IO.Class
+#ifndef mingw32_HOST_OS
+import System.Posix.Temp (mkdtemp)
+#endif
 
 import Utility.Exception
 import Utility.FileSystemEncoding
@@ -64,25 +67,22 @@ withTmpFileIn tmpdir template a = bracket create remove use
  - directory and all its contents. -}
 withTmpDir :: (MonadMask m, MonadIO m) => Template -> (FilePath -> m a) -> m a
 withTmpDir template a = do
-	tmpdir <- liftIO $ catchDefaultIO "." getTemporaryDirectory
-	withTmpDirIn tmpdir template a
+	topleveltmpdir <- liftIO $ catchDefaultIO "." getTemporaryDirectory
+#ifndef mingw32_HOST_OS
+	-- Use mkdtemp to create a temp directory securely in /tmp.
+	bracket
+		(liftIO $ mkdtemp $ topleveltmpdir </> template)
+		removeTmpDir
+		a
+#else
+	withTmpDirIn topleveltmpdir template a
+#endif
 
 {- Runs an action with a tmp directory located within a specified directory,
  - then removes the tmp directory and all its contents. -}
 withTmpDirIn :: (MonadMask m, MonadIO m) => FilePath -> Template -> (FilePath -> m a) -> m a
-withTmpDirIn tmpdir template = bracketIO create remove
+withTmpDirIn tmpdir template = bracketIO create removeTmpDir
   where
-	remove d = whenM (doesDirectoryExist d) $ do
-#if mingw32_HOST_OS
-		-- Windows will often refuse to delete a file
-		-- after a process has just written to it and exited.
-		-- Because it's crap, presumably. So, ignore failure
-		-- to delete the temp directory.
-		_ <- tryIO $ removeDirectoryRecursive d
-		return ()
-#else
-		removeDirectoryRecursive d
-#endif
 	create = do
 		createDirectoryIfMissing True tmpdir
 		makenewdir (tmpdir </> template) (0 :: Int)
@@ -92,6 +92,21 @@ withTmpDirIn tmpdir template = bracketIO create remove
 			createDirectory dir
 			return dir
 
+{- Deletes the entire contents of the the temporary directory, if it
+ - exists. -}
+removeTmpDir :: MonadIO m => FilePath -> m ()
+removeTmpDir tmpdir = liftIO $ whenM (doesDirectoryExist tmpdir) $ do
+#if mingw32_HOST_OS
+	-- Windows will often refuse to delete a file
+	-- after a process has just written to it and exited.
+	-- Because it's crap, presumably. So, ignore failure
+	-- to delete the temp directory.
+	_ <- tryIO $ removeDirectoryRecursive tmpdir
+	return ()
+#else
+	removeDirectoryRecursive tmpdir
+#endif
+
 {- It's not safe to use a FilePath of an existing file as the template
  - for openTempFile, because if the FilePath is really long, the tmpfile
  - will be longer, and may exceed the maximum filename length.
diff --git a/debian/changelog b/debian/changelog
index a2c59f005..7fc9a1cff 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -5,6 +5,9 @@ git-annex (5.20151209) UNRELEASED; urgency=medium
     The empty file it was sending tickled bugs in some php WebDAV server.
   * fsck: Failed to honor annex.diskreserve when checking a remote.
   * Debian: Build depend on concurrent-output.
+  * Fix insecure temporary permissions when git-annex repair is used in
+    in a corrupted git repository.
+  * Fix potential denial of service attack when creating temp dirs.
 
  -- Joey Hess <id@joeyh.name>  Thu, 10 Dec 2015 11:39:34 -0400