diff --git a/src/ICSharpCode.SharpZipLib/Core/InvalidNameException.cs b/src/ICSharpCode.SharpZipLib/Core/InvalidNameException.cs new file mode 100644 index 000000000..99cc0e7e3 --- /dev/null +++ b/src/ICSharpCode.SharpZipLib/Core/InvalidNameException.cs @@ -0,0 +1,38 @@ +using System; +using System.Collections.Generic; +using System.Text; + +namespace ICSharpCode.SharpZipLib.Core +{ + + /// + /// InvalidNameException is thrown for invalid names such as directory traversal paths and names with invalid characters + /// + public class InvalidNameException: SharpZipBaseException + { + /// + /// Initializes a new instance of the InvalidNameException class with a default error message. + /// + public InvalidNameException(): base("An invalid name was specified") + { + } + + /// + /// Initializes a new instance of the InvalidNameException class with a specified error message. + /// + /// A message describing the exception. + public InvalidNameException(string message) : base(message) + { + } + + /// + /// 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. + /// + /// A message describing the exception. + /// The inner exception + public InvalidNameException(string message, Exception innerException) : base(message, innerException) + { + } + } +} diff --git a/src/ICSharpCode.SharpZipLib/Zip/FastZip.cs b/src/ICSharpCode.SharpZipLib/Zip/FastZip.cs index 14ef5443b..c58b4ccf2 100644 --- a/src/ICSharpCode.SharpZipLib/Zip/FastZip.cs +++ b/src/ICSharpCode.SharpZipLib/Zip/FastZip.cs @@ -385,12 +385,13 @@ public void ExtractZip(string zipFileName, string targetDirectory, string fileFi /// A filter to apply to files. /// A filter to apply to directories. /// Flag indicating whether to restore the date and time for extracted files. + /// Allow parent directory traversal in file paths (e.g. ../file) 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); } /// @@ -404,10 +405,11 @@ public void ExtractZip(string zipFileName, string targetDirectory, /// A filter to apply to directories. /// Flag indicating whether to restore the date and time for extracted files. /// Flag indicating whether the inputStream will be closed by this method. + /// Allow parent directory traversal in file paths (e.g. ../file) 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); diff --git a/src/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs b/src/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs index 2dd32f83c..5ae841527 100644 --- a/src/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs +++ b/src/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs @@ -19,6 +19,7 @@ public class WindowsNameTransform : INameTransform string _baseDirectory; bool _trimIncomingPaths; char _replacementChar = '_'; + private bool _allowParentTraversal; /// /// In this case we need Windows' invalid path characters. @@ -38,13 +39,11 @@ public class WindowsNameTransform : INameTransform /// Initialises a new instance of /// /// - public WindowsNameTransform(string baseDirectory) + /// Allow parent directory traversal in file paths (e.g. ../file) + 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; } /// @@ -69,6 +68,15 @@ public string BaseDirectory { } } + /// + /// Allow parent directory traversal in file paths (e.g. ../file) + /// + public bool AllowParentTraversal + { + get => _allowParentTraversal; + set => _allowParentTraversal = value; + } + /// /// Gets or sets a value indicating wether paths on incoming values should be removed. /// @@ -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; diff --git a/test/ICSharpCode.SharpZipLib.Tests/Zip/FastZipHandling.cs b/test/ICSharpCode.SharpZipLib.Tests/Zip/FastZipHandling.cs index 8044b6dd2..13c7232d1 100644 --- a/test/ICSharpCode.SharpZipLib.Tests/Zip/FastZipHandling.cs +++ b/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(() => { + 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); + } + } + } }