Skip to content

Instantly share code, notes, and snippets.

@thomas-chauchefoin-bentley-systems
Last active November 19, 2024 07:25
Show Gist options
  • Save thomas-chauchefoin-bentley-systems/855218959116f870f08857cce2aec731 to your computer and use it in GitHub Desktop.
Save thomas-chauchefoin-bentley-systems/855218959116f870f08857cce2aec731 to your computer and use it in GitHub Desktop.
CVE-2024-48510

CVE-2024-48510

Suggested CVSS : 8.6 / High (CVSS:4.0/AV:L/AC:L/AT:N/PR:N/UI:N/VC:H/VI:H/VA:H/SC:N/SI:N/SA:N)

Affected product and versions

DotNetZip >= 1.10.1 (May 2018), <= 1.16.0 (latest release) (NuGet, GitHub)

Description

DotNetZip is affected by an absolute path traversal vulnerability. Despite the protection in place during extraction, the destination path of the current ZIP entry is mutated after its sanitization, breaking security guarantees. This is effectively a bypass of the patch for CVE-2018-1002205.

Subsequently to our report to the upstream maintainer and Microsoft, the repository was marked as read-only and the package legacy on NuGet. The package is still available for download and will not receive a patch, making it a "forever-day".

We recommend migrating code to System.IO.Compression, and attached a patch for DotNetZip to fix this bug during the transition period.

Credits

Finder, Remediation Developer: Thomas Chauchefoin (Bentley Systems)

From 67dcc283bc6a51833fb11fa0b1ac210c55f88ea4 Mon Sep 17 00:00:00 2001
From: Thomas Chauchefoin <thomas.chauchefoin@bentley.com>
Date: Tue, 28 May 2024 00:12:55 +0200
Subject: [PATCH] Prevent absolute path traversal bugs when extracting ZIP
archives
After constructing the path where to extract the current entry, get its fully
qualified form and ensure it stays below the intended base directory.
---
src/Zip Tests/Compatibility.cs | 8 ++++++++
src/Zip Tests/Zip Tests.csproj | 3 +++
src/Zip Tests/zips/absolute-path-traversal.zip | Bin 0 -> 143 bytes
src/Zip.Shared/ZipEntry.Extract.cs | 9 +++++++++
4 files changed, 20 insertions(+)
create mode 100644 src/Zip Tests/zips/absolute-path-traversal.zip
diff --git a/src/Zip Tests/Compatibility.cs b/src/Zip Tests/Compatibility.cs
index d09e43f..4864843 100644
--- a/src/Zip Tests/Compatibility.cs
+++ b/src/Zip Tests/Compatibility.cs
@@ -2605,6 +2605,14 @@ public void Extract_ZipWithRelativePathsInSubdirOutside()
Assert.IsTrue(File.Exists(@"extract\Temp\evil.txt"));
}
+ [TestMethod]
+ [ExpectedException(typeof(IOException))]
+ public void Extract_ZipWithAbsolutePathsOutside()
+ {
+ _Extract_ZipFile("absolute-path-traversal.zip");
+ Assert.IsFalse(File.Exists(@"C:\Windows\Temp\foo"));
+ }
+
private void _Extract_ZipFile(string fileName)
{
TestContext.WriteLine("Current Dir: {0}", CurrentDir);
diff --git a/src/Zip Tests/Zip Tests.csproj b/src/Zip Tests/Zip Tests.csproj
index 8c7be4b..d0fdfdc 100644
--- a/src/Zip Tests/Zip Tests.csproj
+++ b/src/Zip Tests/Zip Tests.csproj
@@ -165,6 +165,9 @@
<Content Include="zips\relative-paths-in-subdir-outside.zip">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
</Content>
+ <Content Include="zips\absolute-path-traversal.zip">
+ <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+ </Content>
</ItemGroup>
<ItemGroup>
<Content Include="zips\winzip-sfx.exe">
diff --git a/src/Zip Tests/zips/absolute-path-traversal.zip b/src/Zip Tests/zips/absolute-path-traversal.zip
new file mode 100644
index 0000000000000000000000000000000000000000..ac62b13709c1f4795e5d56502e763b983c46ee07
GIT binary patch
literal 143
zcmWIWW@Zs#0D+b%yCYWh{4Zk$vO!oBh@GvRt@Oh)^HTE5i}gcNa|`s-^7E4tivqkE
inM4?H>jP>9g9b(rh1HY*Z&o&tBqI<y18Fl5hXDZol^o6h
literal 0
HcmV?d00001
diff --git a/src/Zip.Shared/ZipEntry.Extract.cs b/src/Zip.Shared/ZipEntry.Extract.cs
index 945de82..2131673 100644
--- a/src/Zip.Shared/ZipEntry.Extract.cs
+++ b/src/Zip.Shared/ZipEntry.Extract.cs
@@ -1390,6 +1390,15 @@ bool IsDoneWithOutputToBaseDir(string baseDir, out string outFileName)
// workitem 10639
outFileName = outFileName.Replace('/', Path.DirectorySeparatorChar);
+ // Resolve any directory traversal sequence and compare the result with the intended base directory
+ // where the file or folder will be created.
+ var canonicalOutPath = Path.GetFullPath(outFileName);
+ var canonicalBaseDir = Path.GetFullPath(baseDir);
+ if (!canonicalOutPath.StartsWith(canonicalBaseDir, StringComparison.OrdinalIgnoreCase))
+ {
+ throw new IOException(string.Format("Extracting {0} would write to {1}, outside of {2}; rejecting.", outFileName, canonicalOutPath, canonicalBaseDir));
+ }
+
// check if it is a directory
if (IsDirectory || FileName.EndsWith("/"))
{
--
2.39.2
@mgexo
Copy link

mgexo commented Nov 14, 2024

Regarding StartsWith for such purposes, it is recommended to use StringComparison.OrdinalIgnoreCase
https://blog.mindedsecurity.com/2018/10/how-to-prevent-path-traversal-in-net.html
https://www.stackhawk.com/blog/net-path-traversal-guide-examples-and-prevention/

It may not make difference here but potential utf-8 / encoding attacks would be avoided (some colleagues directly recommended that change when we reviewed the patch)

@thomas-chauchefoin-bentley-systems

Regarding StartsWith for such purposes, it is recommended to use StringComparison.OrdinalIgnoreCase https://blog.mindedsecurity.com/2018/10/how-to-prevent-path-traversal-in-net.html https://www.stackhawk.com/blog/net-path-traversal-guide-examples-and-prevention/

It may not make difference here but potential utf-8 / encoding attacks would be avoided (some colleagues directly recommended that change when we reviewed the patch)

Good point; I updated the patch to include it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment