From fcd731c545de94b277eb2a85ce20317e37ec9030 Mon Sep 17 00:00:00 2001
From: Joey Hess <joeyh@joeyh.name>
Date: Tue, 15 Dec 2015 20:43:44 -0400
Subject: improve temp dir security

http://bugs.debian.org/807341

* Fix insecure temporary permissions.

  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.
---
 Utility/Tmp.hs   | 49 +++++++++++++++++++++++++++++++++----------------
 debian/changelog |  2 ++
 2 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/Utility/Tmp.hs b/Utility/Tmp.hs
index dc55981..7610f6c 100644
--- a/Utility/Tmp.hs
+++ b/Utility/Tmp.hs
@@ -6,6 +6,7 @@
  -}
 
 {-# LANGUAGE CPP #-}
+{-# OPTIONS_GHC -fno-warn-tabs #-}
 
 module Utility.Tmp where
 
@@ -14,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
@@ -63,32 +67,45 @@ 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)
 	makenewdir t n = do
 		let dir = t ++ "." ++ show n
-		either (const $ makenewdir t $ n + 1) (const $ return dir)
-			=<< tryIO (createDirectory dir)
+		catchIOErrorType AlreadyExists (const $ makenewdir t $ n + 1) $ do
+			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
diff --git a/debian/changelog b/debian/changelog
index 3e1df9a..60ff55d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,5 +1,7 @@
 git-repair (1.20150107) UNRELEASED; urgency=medium
 
+  * Fix insecure temporary permissions and potential denial of
+    service attack when creating temp dirs. Closes: #807341
   * Merge from git-annex.
 
  -- Joey Hess <id@joeyh.name>  Wed, 29 Apr 2015 14:59:40 -0400
-- 
cgit v1.2.3