PR #539: Use securely generated random temporary file names

- Use GetRandomFileName() instead of  GetTempFileName()
- Unify random file name creation into PathUtils
- Do not create new random files before use
- Rename  WindowsPathUtils to PathUtils and make a proper static class

Author: piksel

src/ICSharpCode.SharpZipLib/Core/WindowsPathUtils.cs renamed to src/ICSharpCode.SharpZipLib/Core/PathUtils.cs

@@ -1,23 +1,18 @@
+using System.IO;
+
 namespace ICSharpCode.SharpZipLib.Core
 {
 	/// <summary>
-	/// WindowsPathUtils provides simple utilities for handling windows paths.
+	/// PathUtils provides simple utilities for handling paths.
 	/// </summary>
-	public abstract class WindowsPathUtils
+	public static class PathUtils
 	{
-		/// <summary>
-		/// Initializes a new instance of the <see cref="WindowsPathUtils"/> class.
-		/// </summary>
-		internal WindowsPathUtils()
-		{
-		}
-
 		/// <summary>
 		/// Remove any path root present in the path
 		/// </summary>
 		/// <param name="path">A <see cref="string"/> containing path information.</param>
 		/// <returns>The path with the root removed if it was present; path otherwise.</returns>
-		/// <remarks>Unlike the <see cref="System.IO.Path"/> class the path isnt otherwise checked for validity.</remarks>
+		/// <remarks>Unlike the <see cref="System.IO.Path"/> class the path isn't otherwise checked for validity.</remarks>
 		public static string DropPathRoot(string path)
 		{
 			string result = path;
@@ -63,5 +58,25 @@ public static string DropPathRoot(string path)
 			}
 			return result;
 		}
+
+		/// <summary>
+		/// Returns a random file name in the users temporary directory, or in directory of <paramref name="original"/> if specified
+		/// </summary>
+		/// <param name="original">If specified, used as the base file name for the temporary file</param>
+		/// <returns>Returns a temporary file name</returns>
+		public static string GetTempFileName(string original)
+		{
+			string fileName;
+			var tempPath = Path.GetTempPath();
+
+			do
+			{
+				fileName = original == null
+					? Path.Combine(tempPath, Path.GetRandomFileName())
+					: $"{original}.{Path.GetRandomFileName()}";
+			} while (File.Exists(fileName));
+
+			return fileName;
+		}
 	}
 }

src/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs

@@ -176,7 +176,7 @@ public static string MakeValidName(string name, char replacement)
 				throw new ArgumentNullException(nameof(name));
 			}
 
-			name = WindowsPathUtils.DropPathRoot(name.Replace("/", Path.DirectorySeparatorChar.ToString()));
+			name = PathUtils.DropPathRoot(name.Replace("/", Path.DirectorySeparatorChar.ToString()));
 
 			// Drop any leading slashes.
 			while ((name.Length > 0) && (name[0] == Path.DirectorySeparatorChar))

src/ICSharpCode.SharpZipLib/Zip/ZipFile.cs

@@ -4622,18 +4622,8 @@ public DiskArchiveStorage(ZipFile file)
 		/// <returns>Returns the temporary output stream.</returns>
 		public override Stream GetTemporaryOutput()
 		{
-			if (temporaryName_ != null)
-			{
-				temporaryName_ = GetTempFileName(temporaryName_, true);
-				temporaryStream_ = File.Open(temporaryName_, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None);
-			}
-			else
-			{
-				// Determine where to place files based on internal strategy.
-				// Currently this is always done in system temp directory.
-				temporaryName_ = Path.GetTempFileName();
-				temporaryStream_ = File.Open(temporaryName_, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None);
-			}
+			temporaryName_ = PathUtils.GetTempFileName(temporaryName_);
+			temporaryStream_ = File.Open(temporaryName_, FileMode.OpenOrCreate, FileAccess.Write, FileShare.None);
 
 			return temporaryStream_;
 		}
@@ -4652,7 +4642,7 @@ public override Stream ConvertTemporaryToFinal()
 
 			Stream result = null;
 
-			string moveTempName = GetTempFileName(fileName_, false);
+			string moveTempName = PathUtils.GetTempFileName(fileName_);
 			bool newFileCreated = false;
 
 			try
@@ -4691,7 +4681,7 @@ public override Stream MakeTemporaryCopy(Stream stream)
 		{
 			stream.Dispose();
 
-			temporaryName_ = GetTempFileName(fileName_, true);
+			temporaryName_ = PathUtils.GetTempFileName(fileName_);
 			File.Copy(fileName_, temporaryName_, true);
 
 			temporaryStream_ = new FileStream(temporaryName_,
@@ -4741,54 +4731,6 @@ public override void Dispose()
 
 		#endregion IArchiveStorage Members
 
-		#region Internal routines
-
-		private static string GetTempFileName(string original, bool makeTempFile)
-		{
-			string result = null;
-
-			if (original == null)
-			{
-				result = Path.GetTempFileName();
-			}
-			else
-			{
-				int counter = 0;
-				int suffixSeed = DateTime.Now.Second;
-
-				while (result == null)
-				{
-					counter += 1;
-					string newName = string.Format("{0}.{1}{2}.tmp", original, suffixSeed, counter);
-					if (!File.Exists(newName))
-					{
-						if (makeTempFile)
-						{
-							try
-							{
-								// Try and create the file.
-								using (FileStream stream = File.Create(newName))
-								{
-								}
-								result = newName;
-							}
-							catch
-							{
-								suffixSeed = DateTime.Now.Second;
-							}
-						}
-						else
-						{
-							result = newName;
-						}
-					}
-				}
-			}
-			return result;
-		}
-
-		#endregion Internal routines
-
 		#region Instance Fields
 
 		private Stream temporaryStream_;

src/ICSharpCode.SharpZipLib/Zip/ZipNameTransform.cs

@@ -93,7 +93,7 @@ public string TransformFile(string name)
 				}
 
 				name = name.Replace(@"\", "/");
-				name = WindowsPathUtils.DropPathRoot(name);
+				name = PathUtils.DropPathRoot(name);
 
 				// Drop any leading and trailing slashes.
 				name = name.Trim('/');
@@ -289,7 +289,7 @@ public string TransformFile(string name)
 				name = name.Replace(@"\", "/");
 
 				// Remove the path root.
-				name = WindowsPathUtils.DropPathRoot(name);
+				name = PathUtils.DropPathRoot(name);
 
 				// Drop any leading and trailing slashes.
 				name = name.Trim('/');