mirror of
https://github.com/adamhathcock/sharpcompress.git
synced 2026-09-23 07:25:11 +00:00
fix: prevent extracting archived files outside of target path
This PR is meant to fix an arbitrary file write vulnerability, that can be achieved using a specially crafted zip archive, that holds path traversal filenames. When the filename gets concatenated to the target extraction directory, the final path ends up outside of the target folder. A sample malicious zip file named Zip.Evil.zip was used, and when running the code below, resulted in the creation of C:/Temp/evil.txt outside of the intended target directory. There are various possible ways to avoid this issue, some include checking for .. (dot dot) characters in the filename, but the best solution in our opinion is to check if the final target filename, starts with the target folder (after both are resolved to their absolute path). Stay secure, Snyk Team
This commit is contained in:
@@ -48,6 +48,7 @@ namespace SharpCompress.Archives
|
||||
{
|
||||
string destinationFileName;
|
||||
string file = Path.GetFileName(entry.Key);
|
||||
string fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory);
|
||||
|
||||
options = options ?? new ExtractionOptions()
|
||||
{
|
||||
@@ -58,19 +59,35 @@ namespace SharpCompress.Archives
|
||||
if (options.ExtractFullPath)
|
||||
{
|
||||
string folder = Path.GetDirectoryName(entry.Key);
|
||||
string destdir = Path.Combine(destinationDirectory, folder);
|
||||
string destdir = Path.GetFullPath(
|
||||
Path.Combine(fullDestinationDirectoryPath, folder)
|
||||
);
|
||||
|
||||
if (!Directory.Exists(destdir))
|
||||
{
|
||||
if (!destdir.StartsWith(fullDestinationDirectoryPath))
|
||||
{
|
||||
throw new ExtractionException("Entry is trying to create a directory outside of the destination directory.");
|
||||
}
|
||||
|
||||
Directory.CreateDirectory(destdir);
|
||||
}
|
||||
destinationFileName = Path.Combine(destdir, file);
|
||||
}
|
||||
else
|
||||
{
|
||||
destinationFileName = Path.Combine(destinationDirectory, file);
|
||||
destinationFileName = Path.Combine(fullDestinationDirectoryPath, file);
|
||||
}
|
||||
|
||||
if (!entry.IsDirectory)
|
||||
{
|
||||
destinationFileName = Path.GetFullPath(destinationFileName);
|
||||
|
||||
if (!destinationFileName.StartsWith(fullDestinationDirectoryPath))
|
||||
{
|
||||
throw new ExtractionException("Entry is trying to write a file outside of the destination directory.");
|
||||
}
|
||||
|
||||
entry.WriteToFile(destinationFileName, options);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -433,7 +433,34 @@ namespace SharpCompress.Test.Zip
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Zip_Evil_Throws_Exception()
|
||||
{
|
||||
Exception expectedExcetpion = null;
|
||||
string zipFile = Path.Combine(TEST_ARCHIVES_PATH, "Zip.Evil.zip");
|
||||
|
||||
try
|
||||
{
|
||||
using (var archive = ZipArchive.Open(zipFile))
|
||||
{
|
||||
foreach (var entry in archive.Entries.Where(entry => !entry.IsDirectory))
|
||||
{
|
||||
entry.WriteToDirectory(SCRATCH_FILES_PATH, new ExtractionOptions()
|
||||
{
|
||||
ExtractFullPath = true,
|
||||
Overwrite = true
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
expectedExcetpion = ex;
|
||||
}
|
||||
|
||||
Assert.NotEqual(expectedExcetpion, null);
|
||||
}
|
||||
|
||||
class NonSeekableMemoryStream : MemoryStream
|
||||
|
||||
BIN
tests/TestArchives/Archives/Zip.Evil.zip
Normal file
BIN
tests/TestArchives/Archives/Zip.Evil.zip
Normal file
Binary file not shown.
Reference in New Issue
Block a user