This post is part of the series 'Vulnerabilities'. Be sure to check out the rest of the blog posts of the series!
In the previous post, I wrote about zip bombs, which covered the risk of extracting oversized files and the importance of not trusting zip entry headers. The vulnerability described in this post is related: this time it's about path traversal.
A zip file contains a flat list of entries. Each entry includes the file path (e.g. folder/subfolder/sample.txt), the file size, a checksum, and a few other fields. The problem is that the path can be anything. For example, it can be:
- a.txt
- folder/b.txt
- folder/c.txt
- folder/subfolder/c.txt
- ../attack.aspx 👈 This can be dangerous...
If you extract this archive, the file attack.aspx will be placed outside the folder where you extracted the archive. In the context of a website, this means an attacker can replace existing files or add new ones, gaining the same privileges as your website on the server. This makes it possible to execute arbitrary queries against your databases, retrieve secrets from your configuration files, or cause other harm 😈
Here's the code to create a malicious zip archive:
C#
using (var fs = File.OpenWrite("test.zip"))
using (var archive = new ZipArchive(fs, ZipArchiveMode.Create))
{
var entry = archive.CreateEntry("../test.txt", CompressionLevel.NoCompression);
using (var entryStream = entry.Open())
using (var streamWriter = new StreamWriter(entryStream))
{
streamWriter.Write("test");
}
}
When extracting an archive, you concatenate the destination path with the entry path using code similar to Path.Combine(destinationDirectoryFullPath, entry.FullName). You must then verify that the resulting path remains under the destination directory.
C#
static void ExtractRelativeToDirectory(ZipArchive archive, string destinationDirectoryName, bool overwrite)
{
foreach (var entry in archive.Entries)
{
ExtractRelativeToDirectory(entry, destinationDirectoryName, overwrite);
}
}
static void ExtractRelativeToDirectory(ZipArchiveEntry entry, string destinationDirectoryName, bool overwrite)
{
if (entry == null)
throw new ArgumentNullException(nameof(entry));
if (destinationDirectoryName == null)
throw new ArgumentNullException(nameof(destinationDirectoryName));
// Note that this will give us a good DirectoryInfo even if destinationDirectoryName exists:
DirectoryInfo di = Directory.CreateDirectory(destinationDirectoryName);
string destinationDirectoryFullPath = di.FullName;
if (!destinationDirectoryFullPath.EndsWith(Path.DirectorySeparatorChar))
{
destinationDirectoryFullPath += Path.DirectorySeparatorChar;
}
string fileDestinationPath = Path.GetFullPath(Path.Combine(destinationDirectoryFullPath, entry.FullName));
// Ensure we are not extracting a file outside of the destinationDirectoryName
if (!fileDestinationPath.StartsWith(destinationDirectoryFullPath, StringComparison.OrdinalIgnoreCase))
throw new Exception($"entry '{entry.FullName}' is outside of the destination directory");
if (Path.GetFileName(fileDestinationPath).Length == 0)
{
// If it is a directory:
if (entry.Length != 0)
throw new Exception($"The entry '{entry.FullName}' is a directory but contains data");
Directory.CreateDirectory(fileDestinationPath);
}
else
{
// If it is a file:
// Create containing directory:
Directory.CreateDirectory(Path.GetDirectoryName(fileDestinationPath));
entry.ExtractToFile(fileDestinationPath, overwrite: overwrite);
}
}
This code is adapted from the .NET runtime (source), so if you use ZipFile.ExtractToDirectory or ZipArchive.ExtractToDirectory, you are already protected. If you extract files manually, validate the entry paths carefully.
Do you have a question or a suggestion about this post? Contact me!
