diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs index 6b246e4ee4567e..2fa9a65e6e1467 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/Internal/Runtime/CompilerHelpers/InteropHelpers.cs @@ -45,7 +45,9 @@ internal static unsafe void StringToByValAnsiString(string str, byte* pNative, i fixed (char* pManaged = str) { - PInvokeMarshal.StringToAnsiString(pManaged, lenUnicode, pNative, /*terminateWithNull=*/true, bestFit, throwOnUnmappableChar); + PInvokeMarshal.StringToAnsiString(pManaged, lenUnicode, pNative, + /*terminateWithNull=*/true, bestFit, throwOnUnmappableChar, + nativeByteLength: charCount); } } else diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs index 81c9482a557580..3d5de64135b40b 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/InteropServices/PInvokeMarshal.cs @@ -501,9 +501,14 @@ public static unsafe char AnsiCharToWideChar(byte nativeValue) } // c# string (UTF-16) to UTF-8 encoded byte array + // If specified, nativeByteLength represents the length of the output buffer and this function + // will not write more than that many bytes. If negative, this function writes as many bytes + // as needed to encode the string. internal static unsafe byte* StringToAnsiString(char* pManaged, int lenUnicode, byte* pNative, bool terminateWithNull, - bool bestFit, bool throwOnUnmappableChar) + bool bestFit, bool throwOnUnmappableChar, int nativeByteLength = -1) { + Debug.Assert(pNative != null || nativeByteLength == -1, "Native buffer should not be null when nativeByteLength is specified."); + bool allAscii = Ascii.IsValid(new ReadOnlySpan(pManaged, lenUnicode)); int length; @@ -516,6 +521,18 @@ public static unsafe char AnsiCharToWideChar(byte nativeValue) length = GetByteCount(pManaged, lenUnicode); } + // Clamp to nativeByteLength when caller provides a bounded output buffer (ByValTStr). + // For non-ASCII, ConvertWideCharToMultiByte will throw (Unix) or truncate (Windows) + // when the encoded bytes exceed the clamped length. This matches CoreCLR behavior. + if (nativeByteLength >= 0) + { + int maxBytesToWrite = terminateWithNull ? Math.Max(0, nativeByteLength - 1) : nativeByteLength; + if (length > maxBytesToWrite) + { + length = maxBytesToWrite; + } + } + if (pNative == null) { pNative = (byte*)Marshal.AllocCoTaskMem(checked(length + 1)); @@ -535,8 +552,8 @@ public static unsafe char AnsiCharToWideChar(byte nativeValue) throwOnUnmappableChar); } - // Zero terminate - if (terminateWithNull) + // Zero terminate if requested and the buffer is not specified to be size 0. + if (terminateWithNull && nativeByteLength != 0) *(pNative + length) = 0; return pNative; diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs index 8191f0bacd18fe..4883f9775e2d67 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs @@ -351,7 +351,8 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b string? fileDestinationPath = GetFullDestinationPath( destinationDirectoryPath, Path.IsPathFullyQualified(name) ? name : Path.Join(destinationDirectoryPath, name)); - if (fileDestinationPath == null) + + if (fileDestinationPath is null || FilePathEscapesDirectory(destinationDirectoryPath, fileDestinationPath)) { throw new IOException(SR.Format(SR.TarExtractingResultsFileOutside, name, destinationDirectoryPath)); } @@ -372,7 +373,7 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b string? linkDestination = GetFullDestinationPath( destinationDirectoryPath, Path.IsPathFullyQualified(linkName) ? linkName : Path.Join(Path.GetDirectoryName(fileDestinationPath), linkName)); - if (linkDestination is null) + if (linkDestination is null || FilePathEscapesDirectory(destinationDirectoryPath, linkDestination)) { throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath)); } @@ -387,7 +388,7 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b string? linkDestination = GetFullDestinationPath( destinationDirectoryPath, Path.Join(destinationDirectoryPath, linkName)); - if (linkDestination is null) + if (linkDestination is null || FilePathEscapesDirectory(destinationDirectoryPath, linkDestination)) { throw new IOException(SR.Format(SR.TarExtractingResultsLinkOutside, linkName, destinationDirectoryPath)); } @@ -398,6 +399,101 @@ internal Task ExtractRelativeToDirectoryAsync(string destinationDirectoryPath, b return (fileDestinationPath, linkTargetPath); } + // Check if the file destination path or the link target path escapes the destination directory, by walking through the relative path components and resolving symlinks at each step. + private static bool FilePathEscapesDirectory(string destinationDirectoryPath, string fileDestinationPath) + { + // Windows is case insensitive while Linux is case sensitive + // This ensures the comparison is consistent with how the OS would resolve the paths + StringComparison pathComparison = OperatingSystem.IsWindows() + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + + string resolvedDest = ResolvePhysicalPath(destinationDirectoryPath); + string destPrefix = resolvedDest.EndsWith(Path.DirectorySeparatorChar) + ? resolvedDest + : resolvedDest + Path.DirectorySeparatorChar; + + // Normalize file path (resolves .. and . but not symlinks) + string normalizedFile = Path.GetFullPath(fileDestinationPath); + + // 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); + + string current = resolvedDest; + + foreach (string component in components) + { + current = Path.Combine(current, component); + + if (Path.Exists(current)) + { + string? resolved = ResolveSymlink(current); + if (resolved is null) + { + return true; + } + current = resolved; + } + + string normalizedCurrent = Path.GetFullPath(current); + if (!normalizedCurrent.StartsWith(destPrefix, pathComparison) && + !normalizedCurrent.Equals(resolvedDest, pathComparison)) + { + return true; + } + } + + return false; + } + + 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; + } + + // Resolves the full path of the specified path, resolving symlinks at each step. + // This is needed to mitigate malicious entries in the archive that could lead to writing files outside of the intended directory. + private static string ResolvePhysicalPath(string path) + { + string fullPath = Path.GetFullPath(path); + string? root = Path.GetPathRoot(fullPath); + + if (root is null) + { + return fullPath; + } + + string[] components = fullPath.Substring(root.Length) + .Split(new char[] { Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar }, StringSplitOptions.RemoveEmptyEntries); + string current = root; + foreach (string component in components) + { + current = Path.Combine(current, component); + if (Path.Exists(current)) + { + string? resolved = ResolveSymlink(current); + if (resolved is null) + { + return current; + } + current = resolved; + } + } + + return current; + } + // Returns the full destination path if the path is the destinationDirectory or a subpath. Otherwise, returns null. private static string? GetFullDestinationPath(string destinationDirectoryFullPath, string qualifiedPath) { diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs index 9ffbcc00792703..d6483b8444b1c6 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; +using System.Text; using Xunit; namespace System.Formats.Tar.Tests @@ -368,5 +369,94 @@ public void LinkBeforeTarget() Assert.True(File.Exists(filePath), $"{filePath}' does not exist."); Assert.True(File.Exists(linkPath), $"{linkPath}' does not exist."); } + + [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] + public void ExtractToDirectory_RejectsSymlinkDirectoryTraversal_WithNestedFile() + { + using TempDirectory root = new TempDirectory(); + string destDir = Path.Combine(root.Path, "dest"); + Directory.CreateDirectory(destDir); + + // Absolute path outside destDir + string linkTarget = "/tmp/outside"; + + string tarPath = Path.Combine(root.Path, "symlink_dir_traversal.tar"); + using (FileStream stream = new FileStream(tarPath, FileMode.Create, FileAccess.Write)) + using (TarWriter writer = new TarWriter(stream, leaveOpen: false)) + { + // symlink: "link" -> "/tmp/outside" + writer.WriteEntry(new PaxTarEntry(TarEntryType.SymbolicLink, "link") + { + LinkName = linkTarget + }); + + // file: "link/test.txt" with "hello" + byte[] content = Encoding.UTF8.GetBytes("hello"); + var fileEntry = new PaxTarEntry(TarEntryType.RegularFile, "link/test.txt") + { + DataStream = new MemoryStream(content, writable: false) + }; + + fileEntry.DataStream.Position = 0; + writer.WriteEntry(fileEntry); + } + + Assert.Throws(() => TarFile.ExtractToDirectory(tarPath, destDir, overwriteFiles: true)); + + // Nothing should be created in dest + string linkPath = Path.Combine(destDir, "link"); + 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."); + } + + + [ConditionalFact(typeof(MountHelper), nameof(MountHelper.CanCreateSymbolicLinks))] + public void ExtractToDirectory_RejectsChainedSymlinkDirectoryTraversal_WithNestedFile() + { + // dir a/ + // symlink a/b → . + // symlink a/b/c → . + // symlink a/b/c/d → ../../outside + // file a/d/ pwned.txt escapes + + using TempDirectory root = new TempDirectory(); + string destDir = Path.Combine(root.Path, "dest"); + Directory.CreateDirectory(destDir); + + string tarPath = Path.Combine(root.Path, "chained_symlink_traversal.tar"); + using (FileStream stream = new FileStream(tarPath, FileMode.Create, FileAccess.Write)) + using (TarWriter writer = new TarWriter(stream, leaveOpen: false)) + { + writer.WriteEntry(new PaxTarEntry(TarEntryType.Directory, "a/")); + + writer.WriteEntry(new PaxTarEntry(TarEntryType.SymbolicLink, "a/b") { LinkName = "." }); + + writer.WriteEntry(new PaxTarEntry(TarEntryType.SymbolicLink, "a/b/c") { LinkName = "." }); + + 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")) + }; + writer.WriteEntry(pwned); + } + + if (OperatingSystem.IsWindows()) + { + // Windows only creates file symlinks and trying to process a directory symlink will throw UnauthorizedAccessException instead of IOException + Assert.Throws(() => TarFile.ExtractToDirectory(tarPath, destDir, overwriteFiles: true)); + } + else + { + Assert.Throws(() => TarFile.ExtractToDirectory(tarPath, destDir, overwriteFiles: true)); + } + + string outsideDir = Path.Combine(root.Path, "outside"); + Assert.False(Directory.Exists(outsideDir), "outside/directory should not have been created."); + Assert.False(File.Exists(Path.Combine(outsideDir, "pwned.txt")), "pwned.txt should not have been written outside destination."); + + } } } diff --git a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs index 59cefc137cef81..4a906e0fc6be1a 100644 --- a/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs +++ b/src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/StructureToPtrTests.cs @@ -363,5 +363,58 @@ public struct InnerStruct public InnerStruct s; public byte b; } + + [StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)] + public struct StructWithByValString + { + [MarshalAs(UnmanagedType.ByValTStr, SizeConst = 4)] + public string? Name; + } + + [Fact] + [PlatformSpecific(TestPlatforms.AnyUnix)] + public void StructureToPtr_ByValTStr_MultiByte_Overflow() + { + // ByValTStr uses UTF-8 on Unix. € is 3 bytes, so the string exceeds the specified SizeConst + var payload = new StructWithByValString { Name = "€€€" }; + + int size = Marshal.SizeOf(); + IntPtr buffer = Marshal.AllocHGlobal(size + 1); + byte sentinelValue = 0xFF; + try + { + Marshal.WriteByte(buffer, size, sentinelValue); + Assert.Throws(() => Marshal.StructureToPtr(payload, buffer, false)); + Assert.Equal(sentinelValue, Marshal.ReadByte(buffer, size)); + } + finally + { + Marshal.FreeHGlobal(buffer); + } + } + + [Fact] + public void StructureToPtr_ByValTStr_Ascii_TruncatesLongString() + { + var payload = new StructWithByValString { Name = "abcdef" }; + + int size = Marshal.SizeOf(); + IntPtr buffer = Marshal.AllocHGlobal(size + 1); + byte sentinelValue = 0xFF; + try + { + Marshal.WriteByte(buffer, size, sentinelValue); + Marshal.StructureToPtr(payload, buffer, false); + Assert.Equal((byte)'a', Marshal.ReadByte(buffer, 0)); + Assert.Equal((byte)'b', Marshal.ReadByte(buffer, 1)); + Assert.Equal((byte)'c', Marshal.ReadByte(buffer, 2)); + Assert.Equal((byte)0, Marshal.ReadByte(buffer, 3)); + Assert.Equal(sentinelValue, Marshal.ReadByte(buffer, size)); + } + finally + { + Marshal.FreeHGlobal(buffer); + } + } } }