Restrict path traversal on FastZip extraction (fixes #232)

src/ICSharpCode.SharpZipLib/Core/InvalidNameException.cs

@@ -0,0 +1,38 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace ICSharpCode.SharpZipLib.Core
+{
+
+ /// <summary>
+ /// InvalidNameException is thrown for invalid names such as directory traversal paths and names with invalid characters
+ /// </summary>
+ public class InvalidNameException: SharpZipBaseException
+ {
+ /// <summary>
+ /// Initializes a new instance of the InvalidNameException class with a default error message.
+ /// </summary>
+ public InvalidNameException(): base("An invalid name was specified")
+ {
+ }
+
+ /// <summary>
+ /// Initializes a new instance of the InvalidNameException class with a specified error message.
+ /// </summary>
+ /// <param name="message">A message describing the exception.</param>
+ public InvalidNameException(string message) : base(message)
+ {
+ }
+
+ /// <summary>
+ /// Initializes a new instance of the InvalidNameException class with a specified
+ /// error message and a reference to the inner exception that is the cause of this exception.
+ /// </summary>
+ /// <param name="message">A message describing the exception.</param>
+ /// <param name="innerException">The inner exception</param>
+ public InvalidNameException(string message, Exception innerException) : base(message, innerException)
+ {
+ }
+ }
+}

src/ICSharpCode.SharpZipLib/Zip/FastZip.cs

@@ -385,12 +385,13 @@ public void ExtractZip(string zipFileName, string targetDirectory, string fileFi
  /// <param name="fileFilter">A filter to apply to files.</param>
  /// <param name="directoryFilter">A filter to apply to directories.</param>
  /// <param name="restoreDateTime">Flag indicating whether to restore the date and time for extracted files.</param>
+ /// <param name="allowParentTraversal">Allow parent directory traversal in file paths (e.g. ../file)</param>
  public void ExtractZip(string zipFileName, string targetDirectory,
  Overwrite overwrite, ConfirmOverwriteDelegate confirmDelegate,
- string fileFilter, string directoryFilter, bool restoreDateTime)
+ string fileFilter, string directoryFilter, bool restoreDateTime, bool allowParentTraversal = false)
  {
  Stream inputStream = File.Open(zipFileName, FileMode.Open, FileAccess.Read, FileShare.Read);
- ExtractZip(inputStream, targetDirectory, overwrite, confirmDelegate, fileFilter, directoryFilter, restoreDateTime, true);
+ ExtractZip(inputStream, targetDirectory, overwrite, confirmDelegate, fileFilter, directoryFilter, restoreDateTime, true, allowParentTraversal);
  }
 
  /// <summary>
@@ -404,10 +405,11 @@ public void ExtractZip(string zipFileName, string targetDirectory,
  /// <param name="directoryFilter">A filter to apply to directories.</param>
  /// <param name="restoreDateTime">Flag indicating whether to restore the date and time for extracted files.</param>
  /// <param name="isStreamOwner">Flag indicating whether the inputStream will be closed by this method.</param>
+ /// <param name="allowParentTraversal">Allow parent directory traversal in file paths (e.g. ../file)</param>
  public void ExtractZip(Stream inputStream, string targetDirectory,
  Overwrite overwrite, ConfirmOverwriteDelegate confirmDelegate,
  string fileFilter, string directoryFilter, bool restoreDateTime,
- bool isStreamOwner)
+ bool isStreamOwner, bool allowParentTraversal = false)
  {
  if ((overwrite == Overwrite.Prompt) && (confirmDelegate == null)) {
  throw new ArgumentNullException(nameof(confirmDelegate));
@@ -416,7 +418,7 @@ public void ExtractZip(Stream inputStream, string targetDirectory,
  continueRunning_ = true;
  overwrite_ = overwrite;
  confirmDelegate_ = confirmDelegate;
- extractNameTransform_ = new WindowsNameTransform(targetDirectory);
+ extractNameTransform_ = new WindowsNameTransform(targetDirectory, allowParentTraversal);
 
  fileFilter_ = new NameFilter(fileFilter);
  directoryFilter_ = new NameFilter(directoryFilter);

src/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs

@@ -19,6 +19,7 @@ public class WindowsNameTransform : INameTransform
  string _baseDirectory;
  bool _trimIncomingPaths;
  char _replacementChar = '_';
+ private bool _allowParentTraversal;
 
  /// <summary>
  /// In this case we need Windows' invalid path characters.
@@ -38,13 +39,11 @@ public class WindowsNameTransform : INameTransform
  /// Initialises a new instance of <see cref="WindowsNameTransform"/>
  /// </summary>
  /// <param name="baseDirectory"></param>
- public WindowsNameTransform(string baseDirectory)
+ /// <param name="allowParentTraversal">Allow parent directory traversal in file paths (e.g. ../file)</param>
+ public WindowsNameTransform(string baseDirectory, bool allowParentTraversal = false)
  {
- if (baseDirectory == null) {
- throw new ArgumentNullException(nameof(baseDirectory), "Directory name is invalid");
- }
-
- BaseDirectory = baseDirectory;
+ BaseDirectory = baseDirectory ?? throw new ArgumentNullException(nameof(baseDirectory), "Directory name is invalid");
+ AllowParentTraversal = allowParentTraversal;
  }
 
  /// <summary>
@@ -69,6 +68,15 @@ public string BaseDirectory {
  }
  }
 
+ /// <summary>
+ /// Allow parent directory traversal in file paths (e.g. ../file)
+ /// </summary>
+ public bool AllowParentTraversal
+ {
+ get => _allowParentTraversal;
+ set => _allowParentTraversal = value;
+ }
+
  /// <summary>
  /// Gets or sets a value indicating wether paths on incoming values should be removed.
  /// </summary>
@@ -90,7 +98,7 @@ public string TransformDirectory(string name)
  name = name.Remove(name.Length - 1, 1);
  }
  } else {
- throw new ZipException("Cannot have an empty directory name");
+ throw new InvalidNameException("Cannot have an empty directory name");
  }
  return name;
  }
@@ -113,6 +121,11 @@ public string TransformFile(string name)
  // Combine will throw a PathTooLongException in that case.
  if (_baseDirectory != null) {
  name = Path.Combine(_baseDirectory, name);
+
+ if(!_allowParentTraversal && !Path.GetFullPath(name).StartsWith(_baseDirectory, StringComparison.InvariantCultureIgnoreCase))
+ {
+ throw new InvalidNameException("Parent traversal in paths is not allowed");
+ }
  }
  } else {
  name = string.Empty;

test/ICSharpCode.SharpZipLib.Tests/Zip/FastZipHandling.cs

@@ -1,4 +1,5 @@
-using System.IO;
+using System;
+using System.IO;
 using System.Text.RegularExpressions;
 using ICSharpCode.SharpZipLib.Zip;
 using NUnit.Framework;
@@ -269,5 +270,78 @@ public void NonAsciiPasswords()
  File.Delete(tempName1);
  }
  }
+
+
+ [Test]
+ [Category("Zip")]
+ [Category("CreatesTempFile")]
+ public void LimitExtractPath()
+ {
+ string tempPath = GetTempFilePath();
+ Assert.IsNotNull(tempPath, "No permission to execute this test?");
+
+ var uniqueName = "SharpZipLib.Test_" + DateTime.Now.Ticks.ToString("x");
+
+ tempPath = Path.Combine(tempPath, uniqueName);
+ var extractPath = Path.Combine(tempPath, "output");
+
+ const string contentFile = "content.txt";
+
+ var contentFilePathBad = Path.Combine("..", contentFile);
+ var extractFilePathBad = Path.Combine(tempPath, contentFile);
+ var archiveFileBad = Path.Combine(tempPath, "test-good.zip");
+
+ var contentFilePathGood = Path.Combine("childDir", contentFile);
+ var extractFilePathGood = Path.Combine(extractPath, contentFilePathGood);
+ var archiveFileGood = Path.Combine(tempPath, "test-bad.zip");
+
+ try
+ {
+ Directory.CreateDirectory(extractPath);
+
+ // Create test input
+ void CreateTestFile(string archiveFile, string contentPath)
+ {
+ using (var zf = ZipFile.Create(archiveFile))
+ {
+ zf.BeginUpdate();
+ zf.Add(new StringMemoryDataSource($"Content of {archiveFile}"), contentPath);
+ zf.CommitUpdate();
+ }
+ }
+
+ CreateTestFile(archiveFileGood, contentFilePathGood);
+ CreateTestFile(archiveFileBad, contentFilePathBad);
+
+ Assert.IsTrue(File.Exists(archiveFileGood), "Good test archive was not created");
+ Assert.IsTrue(File.Exists(archiveFileBad), "Bad test archive was not created");
+
+ var fastZip = new FastZip();
+
+ Assert.DoesNotThrow(() => {
+ fastZip.ExtractZip(archiveFileGood, extractPath, "");
+ }, "Threw exception on good file name");
+
+ Assert.IsTrue(File.Exists(extractFilePathGood), "Good output file not created");
+
+ Assert.Throws<SharpZipLib.Core.InvalidNameException>(() => {
+ fastZip.ExtractZip(archiveFileBad, extractPath, "");
+ }, "No exception was thrown for bad file name");
+
+ Assert.IsFalse(File.Exists(extractFilePathBad), "Bad output file created");
+
+ Assert.DoesNotThrow(() => {
+ fastZip.ExtractZip(archiveFileBad, extractPath, FastZip.Overwrite.Never, null, "", "", true, true);
+ }, "Threw exception on bad file name when traversal explicitly allowed");
+
+ Assert.IsTrue(File.Exists(extractFilePathBad), "Bad output file not created when traversal explicitly allowed");
+
+ }
+ finally
+ {
+ Directory.Delete(tempPath, true);
+ }
+ }
+
  }
 }