From c6e0526c79dafa0f458143df230bcfd2af4c95cf Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 21 Jul 2026 15:56:46 +0200 Subject: [PATCH 1/3] Mention filename validation and permission handling for ZIP and TAR extraction --- .../zip-tar-best-practices/csharp/Program.cs | 48 +++++++++++++++++ docs/standard/io/zip-tar-best-practices.md | 51 ++++++++++++++++++- 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs b/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs index b2c750a9f0788..fad16153faa82 100644 --- a/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs +++ b/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs @@ -71,6 +71,13 @@ void DangerousExtract(string extractDir) } // +bool ValidateName(string name) +{ + // Placeholder for a policy that checks for allowed characters, reserved names, etc. + // For example, you might disallow names with invalid characters or reserved device names. + return !string.IsNullOrWhiteSpace(name) && !name.Contains(".."); +} + // void SafeExtractZip(string archivePath, string destinationDir, long maxTotalSize, long maxEntrySize, int maxEntryCount) @@ -103,6 +110,19 @@ void SafeExtractZip(string archivePath, string destinationDir, if (totalSize > maxTotalSize) throw new InvalidOperationException("Archive exceeds total size limit."); + // The entry Name can contain arbitrary characters. Some characters may not be + // allowed on certain filesystems or have a special meaning. Applications should + // apply their own policies regarding allowed filenames. ValidateName is a placeholder + // for such a policy. + if (!ValidateName(entry.Name)) + { + throw new IOException($"Entry name '{entry.Name}' is not allowed."); + } + + // ExternalAttributes carry permission bits for Unix platforms, by clearing the + // attributes we enforce extraction with default file permissions. + entry.ExternalAttributes = 0; + // Resolve the full destination path using Path.GetFullPath, which // normalizes away any "../" segments. Then verify the result still // starts with the destination directory. @@ -174,6 +194,34 @@ void SafeExtractTar(Stream archiveStream, string destinationDir, if (!allowedTypes.Contains(entry.EntryType)) continue; + // The entry Name can contain arbitrary characters. Some characters may not be + // allowed on certain filesystems or have a special meaning. Applications should + // apply their own policies regarding allowed filenames. ValidateName is a placeholder + // for such a policy. + if (!ValidateName(entry.Name)) + { + throw new IOException($"Entry name '{entry.Name}' is not allowed."); + } + + // Mask the entry's permission bits to a safe subset, this subset may depend on your application's needs. + const UnixFileMode PermittedFileModes = + UnixFileMode.UserRead | UnixFileMode.UserWrite | + UnixFileMode.GroupRead | + UnixFileMode.OtherRead; + + const UnixFileMode PermittedDirectoryModes = + PermittedFileModes | + UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; + + if (entry.EntryType == TarEntryType.Directory) + { + entry.Mode &= PermittedDirectoryModes; + } + else + { + entry.Mode &= PermittedFileModes; + } + // Normalize and validate the path, same as the ZIP example. string destPath = Path.GetFullPath(Path.Join(fullDestDir, entry.Name)); if (!destPath.StartsWith(fullDestDir, StringComparison.Ordinal)) diff --git a/docs/standard/io/zip-tar-best-practices.md b/docs/standard/io/zip-tar-best-practices.md index 93c0beab80dfa..1cdc5c632210b 100644 --- a/docs/standard/io/zip-tar-best-practices.md +++ b/docs/standard/io/zip-tar-best-practices.md @@ -84,8 +84,10 @@ For untrusted input—user uploads, third-party downloads, or network transfers - [What the convenience methods don't protect you from](#what-the-convenience-methods-dont-protect-you-from) - [Enforce size and entry count limits](#enforce-size-and-entry-count-limits) +- [Validate file names](#validate-file-names) - [Validate destination paths](#validate-destination-paths) - [Handle symbolic and hard links (TAR)](#handle-symbolic-and-hard-links-tar) +- [Entry permission bits (Unix only)](#entry-permission-bits) - [Complete safe extraction examples](#complete-safe-extraction-examples) ### What the convenience methods don't protect you from @@ -115,6 +117,10 @@ Neither nor [!TIP] > The same approach applies to TAR archives. Since TAR files are read entry-by-entry via , track both the cumulative data size and entry count as you iterate. +### Validate file names + +Depending on the filesystem, some characters may not be allowed in filenames (or allowed only in certain positions), or have special meaning. Applications should check that the extracted entry names conform to an acceptable pattern. + ### Validate destination paths When you use the streaming APIs, you're responsible for validating every entry's destination path. They perform no path validation at all. @@ -162,6 +168,10 @@ If your use case requires extracting archives with hard links but you want to av For reference, validates both the entry path and link target path against the destination directory boundary. If either resolves outside, an is thrown. rejects symbolic and hard link entries entirely—it throws . +### Entry permission bits (Unix only) + +On Unix-like systems, the convenience APIs apply permission bits from the archive metadata to the extracted file/directory. These permissions may be too broad for the application scenario. E.g. applications may want to prevent extraction of files with executable permissions set. See [Unix file permissions](#unix-file-permissions) later in the document for more details. + ### Complete safe extraction examples Combine path traversal validation, size limits, entry count limits, and link handling in a single extraction loop. @@ -219,7 +229,44 @@ Archive behavior can vary between Windows and Unix. Keep these differences in mi - **ZIP:** Unix permissions are stored in the upper 16 bits of . When extracting on Unix via `ExtractToDirectory` or `ExtractToFile`, the runtime restores ownership permissions (read/write/execute for user/group/other), subject to the process umask. SetUID, SetGID, and StickyBit are stripped. Permissions are not applied if the upper bits are zero. This happens when the ZIP was created on Windows, because .NET on Windows sets `DefaultFileExternalAttributes` to `0`. On Windows, these attributes are always ignored during extraction. - **TAR:** The property represents `UnixFileMode` and can store all 12 permission bits (read/write/execute for user/group/other, plus SetUID, SetGID, and StickyBit). When extracting on Unix via `ExtractToDirectory` or `ExtractToFile`, the runtime applies only the 9 ownership bits (rwx for user/group/other), subject to the process umask. SetUID, SetGID, and StickyBit are stripped for security. -When processing untrusted archives, be aware that extracted files may have executable permissions set by the archive author. Untrusted archives could contain malicious executable files. +When processing untrusted archives, be aware that extracted files may have executable permissions set by the archive author. Untrusted archives could contain malicious executable files. Since and are writable, you can modify them before extraction: + +```csharp +foreach (ZipArchiveEntry entry in archive.Entries) +{ + // unset the external attributes to force extraction with default unix permission bits + entry.ExternalAttributes = 0; + + // .. other validation omitted for brevity + + Directory.CreateDirectory(Path.GetDirectoryName(destPath)!); + entry.ExtractToFile(destPath, overwrite: false); +} +``` + +```csharp + +const UnixFileMode PermittedFileModes = + UnixFileMode.UserRead | UnixFileMode.UserWrite | + UnixFileMode.GroupRead | + UnixFileMode.OtherRead; + +const UnixFileMode PermittedDirectoryModes = + PermittedFileModes | + UnixFileMode.UserExecute | UnixFileMode.GroupExecute | UnixFileMode.OtherExecute; + +while ((entry = reader.GetNextEntry()) is not null) +{ + if (entry.EntryType == TarEntryType.Directory) + { + entry.Mode &= PermittedDirectoryModes; + } + else + { + entry.Mode &= PermittedFileModes; + } +} +``` ### Special entry types (TAR) @@ -333,8 +380,10 @@ Before deploying code that handles archives from untrusted sources, verify you'v - **Manual iteration:** Don't use `ExtractToDirectory` for untrusted input—iterate entries manually to enforce all limits. - **Path traversal:** Validate all destination paths with `Path.GetFullPath()` + `StartsWith()`. +- **Special characters in names:** Validate that entry names conform to the application-defined policy. - **Decompression bombs:** Enforce limits on decompressed size (per-entry and total) and entry count. - **Symlink/hardlink attacks (TAR):** Validate link targets resolve within the destination, or skip link entries entirely. +- **Unix permissions:** Validate each entry's permission bits. Skip entries with too broad permissions or mask/modify the unwanted permission bits before extraction. - **Memory limits:** Avoid for large untrusted archives. Avoid mode with unseekable streams from untrusted sources. - **Thread safety:** Don't share , , or instances across threads. - **Untrusted metadata:** Treat entry names, comments, and extra fields as untrusted input. Sanitize before display or processing. From fba8f7f9c189479fb3d9f7992fb4c9842cc6384d Mon Sep 17 00:00:00 2001 From: Radek Zikmund Date: Tue, 21 Jul 2026 16:37:04 +0200 Subject: [PATCH 2/3] Feedback --- .../io/snippets/zip-tar-best-practices/csharp/Program.cs | 6 +++--- docs/standard/io/zip-tar-best-practices.md | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs b/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs index fad16153faa82..f629a24438e53 100644 --- a/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs +++ b/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs @@ -114,9 +114,9 @@ void SafeExtractZip(string archivePath, string destinationDir, // allowed on certain filesystems or have a special meaning. Applications should // apply their own policies regarding allowed filenames. ValidateName is a placeholder // for such a policy. - if (!ValidateName(entry.Name)) + if (!ValidateName(entry.FullName)) { - throw new IOException($"Entry name '{entry.Name}' is not allowed."); + throw new IOException($"Entry name '{entry.FullName}' is not allowed."); } // ExternalAttributes carry permission bits for Unix platforms, by clearing the @@ -203,7 +203,7 @@ void SafeExtractTar(Stream archiveStream, string destinationDir, throw new IOException($"Entry name '{entry.Name}' is not allowed."); } - // Mask the entry's permission bits to a safe subset, this subset may depend on your application's needs. + // Mask the entry's permission bits to a safe subset. This subset may depend on your application's needs. const UnixFileMode PermittedFileModes = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | diff --git a/docs/standard/io/zip-tar-best-practices.md b/docs/standard/io/zip-tar-best-practices.md index 1cdc5c632210b..4393d1b4ae21e 100644 --- a/docs/standard/io/zip-tar-best-practices.md +++ b/docs/standard/io/zip-tar-best-practices.md @@ -87,7 +87,7 @@ For untrusted input—user uploads, third-party downloads, or network transfers - [Validate file names](#validate-file-names) - [Validate destination paths](#validate-destination-paths) - [Handle symbolic and hard links (TAR)](#handle-symbolic-and-hard-links-tar) -- [Entry permission bits (Unix only)](#entry-permission-bits) +- [Entry permission bits (Unix only)](#entry-permission-bits-unix-only) - [Complete safe extraction examples](#complete-safe-extraction-examples) ### What the convenience methods don't protect you from @@ -170,7 +170,7 @@ For reference, Date: Wed, 22 Jul 2026 11:02:37 +0200 Subject: [PATCH 3/3] Apply suggestions from code review Co-authored-by: Genevieve Warren <24882762+gewarren@users.noreply.github.com> --- .../snippets/zip-tar-best-practices/csharp/Program.cs | 11 ++++++----- docs/standard/io/zip-tar-best-practices.md | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs b/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs index f629a24438e53..2cd9524693b12 100644 --- a/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs +++ b/docs/standard/io/snippets/zip-tar-best-practices/csharp/Program.cs @@ -110,7 +110,7 @@ void SafeExtractZip(string archivePath, string destinationDir, if (totalSize > maxTotalSize) throw new InvalidOperationException("Archive exceeds total size limit."); - // The entry Name can contain arbitrary characters. Some characters may not be + // The entry Name can contain arbitrary characters. Some characters might not be // allowed on certain filesystems or have a special meaning. Applications should // apply their own policies regarding allowed filenames. ValidateName is a placeholder // for such a policy. @@ -119,8 +119,8 @@ void SafeExtractZip(string archivePath, string destinationDir, throw new IOException($"Entry name '{entry.FullName}' is not allowed."); } - // ExternalAttributes carry permission bits for Unix platforms, by clearing the - // attributes we enforce extraction with default file permissions. + // ExternalAttributes carry permission bits for Unix platforms. Clearing the + // attributes enforces extraction with default file permissions. entry.ExternalAttributes = 0; // Resolve the full destination path using Path.GetFullPath, which @@ -194,7 +194,7 @@ void SafeExtractTar(Stream archiveStream, string destinationDir, if (!allowedTypes.Contains(entry.EntryType)) continue; - // The entry Name can contain arbitrary characters. Some characters may not be + // The entry Name can contain arbitrary characters. Some characters might not be // allowed on certain filesystems or have a special meaning. Applications should // apply their own policies regarding allowed filenames. ValidateName is a placeholder // for such a policy. @@ -203,7 +203,8 @@ void SafeExtractTar(Stream archiveStream, string destinationDir, throw new IOException($"Entry name '{entry.Name}' is not allowed."); } - // Mask the entry's permission bits to a safe subset. This subset may depend on your application's needs. + // Mask the entry's permission bits to a safe subset. + // This subset depends on your application's needs. const UnixFileMode PermittedFileModes = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | diff --git a/docs/standard/io/zip-tar-best-practices.md b/docs/standard/io/zip-tar-best-practices.md index 4393d1b4ae21e..b7642267c3d17 100644 --- a/docs/standard/io/zip-tar-best-practices.md +++ b/docs/standard/io/zip-tar-best-practices.md @@ -119,7 +119,7 @@ Neither nor . When extracting on Unix via `ExtractToDirectory` or `ExtractToFile`, the runtime restores ownership permissions (read/write/execute for user/group/other), subject to the process umask. SetUID, SetGID, and StickyBit are stripped. Permissions are not applied if the upper bits are zero. This happens when the ZIP was created on Windows, because .NET on Windows sets `DefaultFileExternalAttributes` to `0`. On Windows, these attributes are always ignored during extraction. - **TAR:** The property represents `UnixFileMode` and can store all 12 permission bits (read/write/execute for user/group/other, plus SetUID, SetGID, and StickyBit). When extracting on Unix via `ExtractToDirectory` or `ExtractToFile`, the runtime applies only the 9 ownership bits (rwx for user/group/other), subject to the process umask. SetUID, SetGID, and StickyBit are stripped for security. -When processing untrusted archives, be aware that extracted files may have executable permissions set by the archive author. Untrusted archives could contain malicious executable files. Since and are writable, you can modify them before extraction: +When processing untrusted archives, be aware that extracted files might have executable permissions set by the archive author. Untrusted archives could contain malicious executable files. Since and are writable, you can modify them before extraction: ```csharp foreach (ZipArchiveEntry entry in archive.Entries) { - // unset the external attributes to force extraction with default unix permission bits + // Unset the external attributes to force extraction with default Unix permission bits. entry.ExternalAttributes = 0; // .. other validation omitted for brevity