Merging internal commits for release/8.0#129195
Open
vseanreesermsft wants to merge 13 commits into
Open
Conversation
…rite for ByValTStr Fix NativeAOT possible out-of-bounds write for ByValTStr StringToByValAnsiString truncated the input string based on Unicode character count only, then called PInvokeMarshal.StringToAnsiString which computed the full ANSI/UTF-8 byte count and wrote it all to the fixed-size native buffer. Add an optional maxByteCount parameter to StringToAnsiString. When set, the byte length is clamped before performing the conversion. For ASCII-only strings, this truncates. For non-ASCII, on Unix, this results in an ArgumentException when the buffer is too small for the encoded bytes and on Windows, this truncates. This matches coreclr. 8.0 version of https://dnceng.visualstudio.com/internal/_git/dotnet-runtime/pullrequest/59956
Solve symlinks before extraction to avoid possible traversals ---- #### AI description (iteration 1) #### PR Classification Security fix to prevent directory traversal attacks via malicious symlinks in tar archives. #### PR Summary This PR fixes a security vulnerability where malicious tar archives could use symlinks to write files outside the intended extraction directory. The fix adds symlink resolution logic that validates paths at each component level during extraction. - `TarEntry.cs`: Added `FilePathEscapesDirectory()`, `ResolveSymlink()`, and `ResolveFullPath()` methods to recursively resolve symlinks and detect directory traversal attempts - `TarEntry.cs`: Enhanced extraction validation to check if file paths and link targets escape the destination directory using the new symlink resolution logic - `TarFile.ExtractToDirectory.File.Tests.cs`: Added tests for nested symlink directory traversal and chained symlink attacks to verify the fix prevents malicious extraction <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot -->
…-merge-8.0-2026-06-09-1512
Contributor
There was a problem hiding this comment.
Pull request overview
This PR merges internal release/8.0 changes that strengthen safety and correctness in two areas: (1) fixed-size ANSI string marshalling for ByValTStr (including NativeAOT), and (2) tar extraction path validation to prevent directory traversal via symlinks. It also adds regression tests to validate the intended behavior.
Changes:
- Add regression tests for
Marshal.StructureToPtrwithByValTStrto verify multibyte overflow behavior on Unix and ASCII truncation behavior. - Harden
System.Formats.Tarextraction by detecting paths that escape the destination directory when intermediate components are symlinks. - Update NativeAOT
StringToAnsiStringto respect a bounded output buffer size forByValTStrscenarios.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs | Adds tests covering ByValTStr overflow (Unix UTF-8 multibyte) and ASCII truncation behavior. |
| src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs | Adds tar extraction tests intended to cover symlink-based traversal cases. |
| src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs | Adds traversal detection that resolves symlinks component-by-component when extracting entries. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs | Adds bounded-buffer support for ANSI marshalling to prevent overwriting fixed buffers. |
| src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs | Routes ByValTStr ANSI marshalling through the new bounded-buffer behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+404
to
+410
| // Walk relative components, resolving symlinks at each step | ||
| string relative = normalizedFile.Substring(resolvedDest.Length) | ||
| .TrimStart(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); | ||
|
|
||
| string[] components = relative.Split(new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, | ||
| StringSplitOptions.RemoveEmptyEntries); | ||
|
|
Comment on lines
+438
to
+448
| private static string? ResolveSymlink(string path) | ||
| { | ||
| FileSystemInfo? target = new FileInfo(path).ResolveLinkTarget(returnFinalTarget: true); | ||
|
|
||
| if (target is null) | ||
| { | ||
| return Path.GetFullPath(path); | ||
| } | ||
|
|
||
| return target.FullName; | ||
| } |
Comment on lines
551
to
+555
| throwOnUnmappableChar); | ||
| } | ||
|
|
||
| // Zero terminate | ||
| if (terminateWithNull) | ||
| // Zero terminate if requested and the buffer is not specified to be size 0. | ||
| if (terminateWithNull && nativeByteLength != 0) |
Comment on lines
+408
to
+410
| string outsideFilePath = Path.Combine(destDir, "link", "test.txt"); | ||
| Assert.False(File.Exists(linkPath) || Directory.Exists(linkPath), "link should not have been created."); | ||
| Assert.False(File.Exists(outsideFilePath) || Directory.Exists(linkPath), "traversal link should not have been created."); |
Comment on lines
+437
to
+441
| writer.WriteEntry(new PaxTarEntry(TarEntryType.SymbolicLink, "a/b/c/d") { LinkName = "../../outside" }); | ||
|
|
||
| var pwned = new PaxTarEntry(TarEntryType.RegularFile, "a/d/pwned.txt") | ||
| { | ||
| DataStream = new MemoryStream(Encoding.UTF8.GetBytes("pwned")) |
This was referenced Jun 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.