Skip to content

Commit

Permalink
Disallow traversal outside of extraction directory
Browse files Browse the repository at this point in the history
Use new parameter allowParentTraversal to re-enable past behaviour
Added new explicit exception for invalid names
Fixes icsharpcode#232
  • Loading branch information
piksel authored and shalupov committed Jun 24, 2018
1 parent 456d615 commit e2727f0
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 12 deletions.
38 changes: 38 additions & 0 deletions 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)
{
}
}
}
3 changes: 2 additions & 1 deletion ICSharpCode.SharpZipLib/ICSharpCode.SharpZipLib.csproj
Expand Up @@ -87,6 +87,7 @@
<Compile Include="Core\FileSystemScanner.cs" />
<Compile Include="Core\INameTransform.cs" />
<Compile Include="Core\IScanFilter.cs" />
<Compile Include="Core\InvalidNameException.cs" />
<Compile Include="Core\NameFilter.cs" />
<Compile Include="Core\PathFilter.cs" />
<Compile Include="Core\StreamUtils.cs" />
Expand Down Expand Up @@ -157,4 +158,4 @@
<Target Name="AfterBuild">
</Target>
-->
</Project>
</Project>
10 changes: 6 additions & 4 deletions ICSharpCode.SharpZipLib/Zip/FastZip.cs
Expand Up @@ -371,12 +371,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>
Expand All @@ -390,10 +391,11 @@ public void ExtractZip(string zipFileName, string targetDirectory, string fileFi
/// <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));
Expand All @@ -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);
Expand Down
27 changes: 20 additions & 7 deletions ICSharpCode.SharpZipLib/Zip/WindowsNameTransform.cs
Expand Up @@ -14,13 +14,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>
Expand All @@ -45,6 +43,15 @@ public WindowsNameTransform()
}
}

/// <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>
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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

Expand Down

0 comments on commit e2727f0

Please sign in to comment.