diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Unix.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Unix.cs index 120f8a41da3893..87a45ba14d6f50 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Unix.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Unix.cs @@ -39,23 +39,5 @@ private void ExtractAsHardLink(string targetFilePath, string hardLinkFilePath) Debug.Assert(!string.IsNullOrEmpty(hardLinkFilePath)); Interop.CheckIo(Interop.Sys.Link(targetFilePath, hardLinkFilePath), hardLinkFilePath); } - - // Unix specific implementation of the method that specifies the file permissions of the extracted file. - private void SetModeOnFile(SafeFileHandle handle) - { - // Only extract USR, GRP, and OTH file permissions, and ignore - // S_ISUID, S_ISGID, and S_ISVTX bits. - // It is off by default because it's possible that a file in an archive could have - // one of these bits set and, unknown to the person extracting, could allow others to - // execute the file as the user or group. - const int ExtractPermissionMask = 0x1FF; - int permissions = (int)Mode & ExtractPermissionMask; - - // If the permissions weren't set at all, don't write the file's permissions. - if (permissions != 0) - { - File.SetUnixFileMode(handle, (UnixFileMode)permissions); - } - } } } diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Windows.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Windows.cs index 491e51d7cd0228..8b0c5ff4a04d22 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Windows.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.Windows.cs @@ -38,13 +38,5 @@ private void ExtractAsHardLink(string targetFilePath, string hardLinkFilePath) Debug.Assert(!string.IsNullOrEmpty(hardLinkFilePath)); Interop.Kernel32.CreateHardLink(hardLinkFilePath, targetFilePath); } - - // Mode is not used on Windows. -#pragma warning disable CA1822 // Member 'SetModeOnFile' does not access instance data and can be marked as static - private void SetModeOnFile(SafeFileHandle handle) -#pragma warning restore CA1822 - { - // TODO: Verify that executables get their 'executable' permission applied on Windows when extracted, if applicable. https://github.com/dotnet/runtime/issues/68230 - } } } 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 d77cd1de184d66..5ee75900cf6e48 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 @@ -541,22 +541,14 @@ private void ExtractAsRegularFile(string destinationFileName) { Debug.Assert(!Path.Exists(destinationFileName)); - FileStreamOptions fileStreamOptions = new() - { - Access = FileAccess.Write, - Mode = FileMode.CreateNew, - Share = FileShare.None, - PreallocationSize = Length, - }; // Rely on FileStream's ctor for further checking destinationFileName parameter - using (FileStream fs = new FileStream(destinationFileName, fileStreamOptions)) + using (FileStream fs = new FileStream(destinationFileName, CreateFileStreamOptions(isAsync: false))) { if (DataStream != null) { // Important: The DataStream will be written from its current position DataStream.CopyTo(fs); } - SetModeOnFile(fs.SafeFileHandle); } ArchivingUtils.AttemptSetLastWriteTime(destinationFileName, ModificationTime); @@ -570,16 +562,8 @@ private async Task ExtractAsRegularFileAsync(string destinationFileName, Cancell cancellationToken.ThrowIfCancellationRequested(); - FileStreamOptions fileStreamOptions = new() - { - Access = FileAccess.Write, - Mode = FileMode.CreateNew, - Share = FileShare.None, - PreallocationSize = Length, - Options = FileOptions.Asynchronous - }; // Rely on FileStream's ctor for further checking destinationFileName parameter - FileStream fs = new FileStream(destinationFileName, fileStreamOptions); + FileStream fs = new FileStream(destinationFileName, CreateFileStreamOptions(isAsync: true)); await using (fs) { if (DataStream != null) @@ -587,10 +571,35 @@ private async Task ExtractAsRegularFileAsync(string destinationFileName, Cancell // Important: The DataStream will be written from its current position await DataStream.CopyToAsync(fs, cancellationToken).ConfigureAwait(false); } - SetModeOnFile(fs.SafeFileHandle); } ArchivingUtils.AttemptSetLastWriteTime(destinationFileName, ModificationTime); } + + private FileStreamOptions CreateFileStreamOptions(bool isAsync) + { + FileStreamOptions fileStreamOptions = new() + { + Access = FileAccess.Write, + Mode = FileMode.CreateNew, + Share = FileShare.None, + PreallocationSize = Length, + Options = isAsync ? FileOptions.Asynchronous : FileOptions.None + }; + + if (!OperatingSystem.IsWindows()) + { + const UnixFileMode OwnershipPermissions = + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute | + UnixFileMode.GroupRead | UnixFileMode.GroupWrite | UnixFileMode.GroupExecute | + UnixFileMode.OtherRead | UnixFileMode.OtherWrite | UnixFileMode.OtherExecute; + + // Restore permissions. + // For security, limit to ownership permissions, and respect umask (through UnixCreateMode). + fileStreamOptions.UnixCreateMode = Mode & OwnershipPermissions; + } + + return fileStreamOptions; + } } } diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj b/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj index b573949d699958..94999191663fad 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj +++ b/src/libraries/System.IO.Compression.ZipFile/src/System.IO.Compression.ZipFile.csproj @@ -24,7 +24,6 @@ - diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.Unix.cs b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.Unix.cs deleted file mode 100644 index adcf474853aa9a..00000000000000 --- a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.Unix.cs +++ /dev/null @@ -1,33 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System.Diagnostics; - -namespace System.IO.Compression -{ - public static partial class ZipFileExtensions - { - static partial void ExtractExternalAttributes(FileStream fs, ZipArchiveEntry entry) - { - Debug.Assert(!OperatingSystem.IsWindows()); - - // Only extract USR, GRP, and OTH file permissions, and ignore - // S_ISUID, S_ISGID, and S_ISVTX bits. This matches unzip's default behavior. - // It is off by default because of this comment: - - // "It's possible that a file in an archive could have one of these bits set - // and, unknown to the person unzipping, could allow others to execute the - // file as the user or group. The new option -K bypasses this check." - const int ExtractPermissionMask = 0x1FF; - int permissions = (entry.ExternalAttributes >> 16) & ExtractPermissionMask; - - // If the permissions weren't set at all, don't write the file's permissions, - // since the .zip could have been made using a previous version of .NET, which didn't - // include the permissions, or was made on Windows. - if (permissions != 0) - { - File.SetUnixFileMode(fs.SafeFileHandle, (UnixFileMode)permissions); - } - } - } -} diff --git a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs index c393c4e1e11c66..f8152033f94775 100644 --- a/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs +++ b/src/libraries/System.IO.Compression.ZipFile/src/System/IO/Compression/ZipFileExtensions.ZipArchiveEntry.Extract.cs @@ -65,22 +65,38 @@ public static void ExtractToFile(this ZipArchiveEntry source, string destination ArgumentNullException.ThrowIfNull(source); ArgumentNullException.ThrowIfNull(destinationFileName); - // Rely on FileStream's ctor for further checking destinationFileName parameter - FileMode fMode = overwrite ? FileMode.Create : FileMode.CreateNew; + FileStreamOptions fileStreamOptions = new() + { + Access = FileAccess.Write, + Mode = overwrite ? FileMode.Create : FileMode.CreateNew, + Share = FileShare.None, + BufferSize = 0x1000 + }; + + const UnixFileMode OwnershipPermissions = + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute | + UnixFileMode.GroupRead | UnixFileMode.GroupWrite | UnixFileMode.GroupExecute | + UnixFileMode.OtherRead | UnixFileMode.OtherWrite | UnixFileMode.OtherExecute; - using (FileStream fs = new FileStream(destinationFileName, fMode, FileAccess.Write, FileShare.None, bufferSize: 0x1000, useAsync: false)) + // Restore Unix permissions. + // For security, limit to ownership permissions, and respect umask (through UnixCreateMode). + // We don't apply UnixFileMode.None because .zip files created on Windows and .zip files created + // with previous versions of .NET don't include permissions. + UnixFileMode mode = (UnixFileMode)(source.ExternalAttributes >> 16) & OwnershipPermissions; + if (mode != UnixFileMode.None && !OperatingSystem.IsWindows()) + { + fileStreamOptions.UnixCreateMode = mode; + } + + using (FileStream fs = new FileStream(destinationFileName, fileStreamOptions)) { using (Stream es = source.Open()) es.CopyTo(fs); - - ExtractExternalAttributes(fs, source); } ArchivingUtils.AttemptSetLastWriteTime(destinationFileName, source.LastWriteTime); } - static partial void ExtractExternalAttributes(FileStream fs, ZipArchiveEntry entry); - internal static void ExtractRelativeToDirectory(this ZipArchiveEntry source, string destinationDirectoryName) => ExtractRelativeToDirectory(source, destinationDirectoryName, overwrite: false); diff --git a/src/libraries/System.IO.Compression.ZipFile/tests/System.IO.Compression.ZipFile.Tests.csproj b/src/libraries/System.IO.Compression.ZipFile/tests/System.IO.Compression.ZipFile.Tests.csproj index 7035c42d23e123..29c88cb2f9ce2e 100644 --- a/src/libraries/System.IO.Compression.ZipFile/tests/System.IO.Compression.ZipFile.Tests.csproj +++ b/src/libraries/System.IO.Compression.ZipFile/tests/System.IO.Compression.ZipFile.Tests.csproj @@ -2,6 +2,7 @@ true true + true $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser diff --git a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs index 2bca01e0c5c00f..25c41ead025d17 100644 --- a/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs +++ b/src/libraries/System.IO.Compression.ZipFile/tests/ZipFile.Unix.cs @@ -5,6 +5,7 @@ using System.Runtime.InteropServices; using System.Text; using System.Threading.Tasks; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.IO.Compression.Tests @@ -56,6 +57,16 @@ void EnsureExternalAttributes(string permissions, ZipArchiveEntry entry) } } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void UnixCreateSetsPermissionsInExternalAttributesUMaskZero() + { + RemoteExecutor.Invoke(() => + { + umask(0); + new ZipFile_Unix().UnixCreateSetsPermissionsInExternalAttributes(); + }).Dispose(); + } + [Fact] public void UnixExtractSetsFilePermissionsFromExternalAttributes() { @@ -90,6 +101,16 @@ public void UnixExtractSetsFilePermissionsFromExternalAttributes() } } + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public void UnixExtractSetsFilePermissionsFromExternalAttributesUMaskZero() + { + RemoteExecutor.Invoke(() => + { + umask(0); + new ZipFile_Unix().UnixExtractSetsFilePermissionsFromExternalAttributes(); + }).Dispose(); + } + private static string[] CreateFiles(string folderPath, string[] testPermissions) { string[] expectedPermissions = new string[testPermissions.Length]; @@ -126,6 +147,8 @@ private static string[] CreateFiles(string folderPath, string[] testPermissions) private static void EnsureFilePermissions(string filename, string permissions) { + permissions = GetExpectedPermissions(permissions); + Interop.Sys.FileStatus status; Assert.Equal(0, Interop.Sys.Stat(filename, out status)); @@ -199,26 +222,28 @@ await Task.WhenAll( private static string GetExpectedPermissions(string expectedPermissions) { - if (string.IsNullOrEmpty(expectedPermissions)) + using (var tempFolder = new TempDirectory()) { - // Create a new file, and get its permissions to get the current system default permissions - - using (var tempFolder = new TempDirectory()) + string filename = Path.Combine(tempFolder.Path, Path.GetRandomFileName()); + FileStreamOptions fileStreamOptions = new() { - string filename = Path.Combine(tempFolder.Path, Path.GetRandomFileName()); - File.WriteAllText(filename, "contents"); - - Interop.Sys.FileStatus status; - Assert.Equal(0, Interop.Sys.Stat(filename, out status)); - - expectedPermissions = Convert.ToString(status.Mode & 0xFFF, 8); + Access = FileAccess.Write, + Mode = FileMode.CreateNew + }; + if (expectedPermissions != null) + { + fileStreamOptions.UnixCreateMode = (UnixFileMode)Convert.ToInt32(expectedPermissions, 8); } - } + new FileStream(filename, fileStreamOptions).Dispose(); - return expectedPermissions; + return Convert.ToString((int)File.GetUnixFileMode(filename), 8); + } } [LibraryImport("libc", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] private static partial int mkfifo(string path, int mode); + + [LibraryImport("libc", StringMarshalling = StringMarshalling.Utf8)] + private static partial int umask(int umask); } }