From 4c45dae897ffff5eb080e5431f89e097e3bf6ea7 Mon Sep 17 00:00:00 2001
From: Max Murphy <maxmrphy@gmail.com>
Date: Sat, 4 Jul 2026 17:03:35 +0200
Subject: [PATCH] fix(OfficeUtils): strengthen ZipSlip path traversal
 protection in ZIP extraction

Replace the weak check that only rejected paths starting with '../'
with proper prefix-based containment validation. The new logic:

- Rejects empty normalized archive entry paths
- Rejects absolute paths in archive entries (Unix / and Windows C:\, \UNC)
- Normalizes both output path and extraction directory via ShortenPath()
  to resolve all ../ and ./ components
- Verifies the resolved output path starts with the extraction directory
  prefix, rejecting any entry that escapes the target directory

Fixes #115

Agentic Model: Qwen3.6-35B-A3B (local)

Signed-off-by: Max Murphy <maxmrphy@gmail.com>
---
 OfficeUtils/src/ZipUtilsCP.cpp | 50 ++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/OfficeUtils/src/ZipUtilsCP.cpp b/OfficeUtils/src/ZipUtilsCP.cpp
index f5fe3ce4ac..58550a30bc 100644
--- a/OfficeUtils/src/ZipUtilsCP.cpp
+++ b/OfficeUtils/src/ZipUtilsCP.cpp
@@ -288,15 +288,55 @@ namespace ZLibZipUtils
 			err = unzOpenCurrentFilePassword(uf, password);
 
 			//-------------------------------------------------------------------------------------------------
-
+			// ZipSlip protection: verify the resolved output path stays within unzip_dir
 			if (unzip_dir)
 			{
-				const std::wstring wsFilePath{NSSystemPath::ShortenPath(filenameW)};
+				// Normalize the filename to resolve any ../ or ./ components
+				const std::wstring wsNormalizedFilename = NSSystemPath::ShortenPath(filenameW);
 
-				if (wsFilePath.size() > 3 && L'.' == wsFilePath[0] && L'.' == wsFilePath[1] && L'/' == wsFilePath[2])
+				// Reject if normalization produced an empty path
+				if (wsNormalizedFilename.empty())
 					return UNZ_INTERNALERROR;
-			}
 
+				// Reject absolute paths in the archive entry itself
+#if defined(_WIN32) || defined (_WIN64)
+				bool bIsAbsolute = false;
+				// Drive letter: C:\ or C:/
+				if (wsNormalizedFilename.size() >= 2 &&
+					wsNormalizedFilename[1] == L':' &&
+					(wsNormalizedFilename[0] >= L'A' && wsNormalizedFilename[0] <= L'Z' ||
+					 wsNormalizedFilename[0] >= L'a' && wsNormalizedFilename[0] <= L'z'))
+					bIsAbsolute = true;
+				// UNC path: \\server\share
+				if (wsNormalizedFilename.size() >= 2 &&
+					wsNormalizedFilename[0] == L'\\' && wsNormalizedFilename[1] == L'\\')
+					bIsAbsolute = true;
+				if (bIsAbsolute)
+					return UNZ_INTERNALERROR;
+#else
+				// Unix-like: absolute path starting with /
+				if (!wsNormalizedFilename.empty() && wsNormalizedFilename[0] == L'/')
+					return UNZ_INTERNALERROR;
+#endif
+
+				// Build the normalized output path and verify containment within unzip_dir
+				std::wstring wsNormalizedOutput = NSSystemPath::ShortenPath(output);
+				std::wstring wsNormalizedDir    = NSSystemPath::ShortenPath(unzip_dir);
+
+				// Ensure directory ends with separator for proper prefix matching
+				if (!wsNormalizedDir.empty() &&
+					wsNormalizedDir.back() != L'/' && wsNormalizedDir.back() != L'\\')
+				{
+					wsNormalizedDir += FILE_SEPARATOR_STR;
+				}
+
+				// Verify the output path starts with the extraction directory prefix
+				if (wsNormalizedOutput.size() < wsNormalizedDir.size() ||
+					wsNormalizedOutput.substr(0, wsNormalizedDir.size()) != wsNormalizedDir)
+				{
+					return UNZ_INTERNALERROR;
+				}
+			}
 			//-------------------------------------------------------------------------------------------------
 			NSFile::CFileBinary oFile;
 			FILE *fout = NULL;
@@ -480,7 +520,7 @@ namespace ZLibZipUtils
 
 		std::wstring filenameW = codepage_issue_fixFromOEM(filename_inzip);
 
-		//TODO is there a need to reduce everything to the bottom???
+		//TODO is there a need to reduce everything to the bottom???
 		if (wcscmp(filename, filenameW.c_str()) == 0)
 			return true;