From e2727f0ee45a53d885ee224bc952205ab7ea759e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nils=20m=C3=A5s=C3=A9n?= Date: Sun, 17 Jun 2018 15:03:17 +0200 Subject: [PATCH] Disallow traversal outside of extraction directory Use new parameter allowParentTraversal to re-enable past behaviour Added new explicit exception for invalid names Fixes #232 --- .../Core/InvalidNameException.cs | 38 +++++++++++++++++++ .../ICSharpCode.SharpZipLib.csproj | 3 +- ICSharpCode.SharpZipLib/Zip/FastZip.cs | 10 +++-- .../Zip/WindowsNameTransform.cs | 27 +++++++++---- 4 files changed, 66 insertions(+), 12 deletions(-) create mode 100644 ICSharpCode.SharpZipLib/Core/InvalidNameException.cs diff --git a/ICSharpCode.SharpZipLib/Core/InvalidNameException.cs b/ICSharpCode.SharpZipLib/Core/InvalidNameException.cs new file mode 100644 index 000000000..99cc0e7e3 --- /dev/null +++ b/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/ICSharpCode.SharpZipLib/ICSharpCode.SharpZipLib.csproj b/ICSharpCode.SharpZipLib/ICSharpCode.SharpZipLib.csproj index 9f6ef7a7c..2a0fb4f88 100644 --- a/ICSharpCode.SharpZipLib/ICSharpCode.SharpZipLib.csproj +++ b/ICSharpCode.SharpZipLib/ICSharpCode.SharpZipLib.csproj @@ -87,6 +87,7 @@ + @@ -157,4 +158,4 @@ --> - \ No newline at end of file + diff --git a/ICSharpCode.SharpZipLib/Zip/FastZip.cs b/ICSharpCode.SharpZipLib/Zip/FastZip.cs index 53e1288df..604394373 100644 --- a/ICSharpCode.SharpZipLib/Zip/FastZip.cs +++ b/ICSharpCode.SharpZipLib/Zip/FastZip.cs @@ -371,12 +371,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); } /// @@ -390,10 +391,11 @@ public void ExtractZip(string zipFileName, string targetDirectory, string fileFi /// 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)); @@ -402,7 +404,7 @@ public void ExtractZip(string zipFileName, string targetDirectory, string fileFi 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/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs b/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs index 663d44ec2..401466882 100644 --- a/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs +++ b/ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs @@ -14,13 +14,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; } /// @@ -45,6 +43,15 @@ public WindowsNameTransform() } } + /// + /// 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. /// @@ -66,7 +73,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; } @@ -89,6 +96,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; @@ -216,6 +228,7 @@ public static string MakeValidName(string name, char replacement) #region Instance Fields string _baseDirectory; bool _trimIncomingPaths; + private bool _allowParentTraversal; char _replacementChar = '_'; #endregion