Skip to content

Commit 80ceb1c

Browse files
committed
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
1 parent 259acd0 commit 80ceb1c

3 files changed

Lines changed: 46 additions & 2 deletions

File tree

src/SharpCompress/Archives/IArchiveEntryExtensions.cs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ public static void WriteToDirectory(this IArchiveEntry entry, string destination
4848
{
4949
string destinationFileName;
5050
string file = Path.GetFileName(entry.Key);
51+
string fullDestinationDirectoryPath = Path.GetFullPath(destinationDirectory);
5152

5253
options = options ?? new ExtractionOptions()
5354
{
@@ -58,19 +59,35 @@ public static void WriteToDirectory(this IArchiveEntry entry, string destination
5859
if (options.ExtractFullPath)
5960
{
6061
string folder = Path.GetDirectoryName(entry.Key);
61-
string destdir = Path.Combine(destinationDirectory, folder);
62+
string destdir = Path.GetFullPath(
63+
Path.Combine(fullDestinationDirectoryPath, folder)
64+
);
65+
6266
if (!Directory.Exists(destdir))
6367
{
68+
if (!destdir.StartsWith(fullDestinationDirectoryPath))
69+
{
70+
throw new ExtractionException("Entry is trying to create a directory outside of the destination directory.");
71+
}
72+
6473
Directory.CreateDirectory(destdir);
6574
}
6675
destinationFileName = Path.Combine(destdir, file);
6776
}
6877
else
6978
{
70-
destinationFileName = Path.Combine(destinationDirectory, file);
79+
destinationFileName = Path.Combine(fullDestinationDirectoryPath, file);
7180
}
81+
7282
if (!entry.IsDirectory)
7383
{
84+
destinationFileName = Path.GetFullPath(destinationFileName);
85+
86+
if (!destinationFileName.StartsWith(fullDestinationDirectoryPath))
87+
{
88+
throw new ExtractionException("Entry is trying to write a file outside of the destination directory.");
89+
}
90+
7491
entry.WriteToFile(destinationFileName, options);
7592
}
7693
}

tests/SharpCompress.Test/Zip/ZipArchiveTests.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,34 @@ public void Zip_Deflate_PKWear_Multipy_Entry_Access()
433433
}
434434
}
435435
}
436+
}
437+
438+
[Fact]
439+
public void Zip_Evil_Throws_Exception()
440+
{
441+
Exception expectedExcetpion = null;
442+
string zipFile = Path.Combine(TEST_ARCHIVES_PATH, "Zip.Evil.zip");
443+
444+
try
445+
{
446+
using (var archive = ZipArchive.Open(zipFile))
447+
{
448+
foreach (var entry in archive.Entries.Where(entry => !entry.IsDirectory))
449+
{
450+
entry.WriteToDirectory(SCRATCH_FILES_PATH, new ExtractionOptions()
451+
{
452+
ExtractFullPath = true,
453+
Overwrite = true
454+
});
455+
}
456+
}
457+
}
458+
catch (Exception ex)
459+
{
460+
expectedExcetpion = ex;
461+
}
436462

463+
Assert.NotEqual(expectedExcetpion, null);
437464
}
438465

439466
class NonSeekableMemoryStream : MemoryStream
547 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)