From 457b0d2baf94809d79c0e5f5770654767a4f5ca5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Mon, 21 Jun 2021 12:54:59 +0200 Subject: [PATCH 1/2] handle UNC and device paths --- .../Common/tests/Common.Tests.csproj | 2 + .../System/IO/PathInternal.Windows.Tests.cs | 43 +++++++++++++++++++ .../SafeHandles/SafeFileHandle.Windows.cs | 27 ++---------- .../src/System/IO/PathInternal.Windows.cs | 30 +++++++++++++ 4 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/libraries/Common/tests/Common.Tests.csproj b/src/libraries/Common/tests/Common.Tests.csproj index ba76497cfc556..cbbf17ff19005 100644 --- a/src/libraries/Common/tests/Common.Tests.csproj +++ b/src/libraries/Common/tests/Common.Tests.csproj @@ -109,6 +109,8 @@ + DosToNtPathTest_Data => new TheoryData + { + { @"C:\tests\file.cs", @"\??\C:\tests\file.cs" }, // typical path + { @"\\?\C:\tests\file.cs", @"\??\C:\tests\file.cs" }, // NtPath with \\?\ prefix + { @"\\.\device\file.cs", @"\??\device\file.cs" }, // device path with \\.\ prefix + { @"\\server\file.cs", @"\??\UNC\server\file.cs" }, // UNC path with \\ prefix + { @"\\?\UNC\server\file", @"\??\UNC\server\file" }, // extended UNC prefix + { @"\??\C:\tests\file.cs", @"\??\C:\tests\file.cs" }, // NtPath with \??\ prefix (no changes required) + { @"C:\", @"\??\C:\" }, // a short path + { @"\\s", @"\??\UNC\s" }, // short UNC path + { @"\\s\", @"\??\UNC\s\" }, // short UNC path with trailing + { $@"C:\{string.Join("\\", Enumerable.Repeat("a", PathInternal.MaxShortPath + 1))}", $@"\??\C:\{string.Join("\\", Enumerable.Repeat("a", PathInternal.MaxShortPath + 1))}"}, // long path + }; + + [Theory, MemberData(nameof(DosToNtPathTest_Data))] + public void DosToNtPathTest(string path, string expected) + { + // first of all, we use an internal Windows API to ensure that expected value is valid + if (path.Length < PathInternal.MaxShortPath) // RtlDosPathNameToRelativeNtPathName_U_WithStatus does not support long paths + { + RtlDosPathNameToRelativeNtPathName_U_WithStatus(path, out Interop.UNICODE_STRING ntFileName, out IntPtr _, IntPtr.Zero); + try + { + Assert.Equal(expected, Marshal.PtrToStringUni(ntFileName.Buffer)); + } + finally + { + Marshal.ZeroFreeGlobalAllocUnicode(ntFileName.Buffer); + } + } + + // after that, we test our implementation + var vsb = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]); + PathInternal.DosToNtPath(path, ref vsb); + Assert.Equal(expected, vsb.ToString()); + + [DllImport(Interop.Libraries.NtDll, CharSet = CharSet.Unicode)] + static extern int RtlDosPathNameToRelativeNtPathName_U_WithStatus(string DosFileName, out Interop.UNICODE_STRING NtFileName, out IntPtr FilePart, IntPtr RelativeName); + } } } diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index 36710dcdbde5d..67b239d2aa0ce 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -56,31 +56,12 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA private static IntPtr NtCreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize) { - uint ntStatus; - IntPtr fileHandle; + var vsb = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]); - const string MandatoryNtPrefix = @"\??\"; - if (fullPath.StartsWith(MandatoryNtPrefix, StringComparison.Ordinal)) - { - (ntStatus, fileHandle) = Interop.NtDll.NtCreateFile(fullPath, mode, access, share, options, preallocationSize); - } - else - { - var vsb = new ValueStringBuilder(stackalloc char[256]); - vsb.Append(MandatoryNtPrefix); + PathInternal.DosToNtPath(fullPath, ref vsb); - if (fullPath.StartsWith(@"\\?\", StringComparison.Ordinal)) // NtCreateFile does not support "\\?\" prefix, only "\??\" - { - vsb.Append(fullPath.AsSpan(4)); - } - else - { - vsb.Append(fullPath); - } - - (ntStatus, fileHandle) = Interop.NtDll.NtCreateFile(vsb.AsSpan(), mode, access, share, options, preallocationSize); - vsb.Dispose(); - } + (uint ntStatus, IntPtr fileHandle) = Interop.NtDll.NtCreateFile(vsb.AsSpan(), mode, access, share, options, preallocationSize); + vsb.Dispose(); switch (ntStatus) { diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs index dd9442b165bce..3ee5bcdab9a29 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs @@ -47,6 +47,7 @@ internal static partial class PathInternal internal const string DirectorySeparatorCharAsString = "\\"; internal const string ExtendedPathPrefix = @"\\?\"; + internal const string NtPrefix = @"\??\"; internal const string UncPathPrefix = @"\\"; internal const string UncExtendedPrefixToInsert = @"?\UNC\"; internal const string UncExtendedPathPrefix = @"\\?\UNC\"; @@ -409,5 +410,34 @@ internal static bool IsEffectivelyEmpty(ReadOnlySpan path) } return true; } + + // this method works only for `fullPath` returned by Path.GetFullPath + // currently we don't have interest in supporting relative paths + internal static void DosToNtPath(ReadOnlySpan fullPath, ref ValueStringBuilder vsb) + { + vsb.Append(NtPrefix); + + if (fullPath.Length >= 3 && fullPath[0] == '\\' && fullPath[1] == '\\') + { + // \\.\ (Device) or \\?\ (NtPath) + if (fullPath.Length >= 4 && fullPath[3] == '\\' && (fullPath[2] == '.' || fullPath[2] == '?')) + { + vsb.Append(fullPath.Slice(NtPrefix.Length)); + } + else // \\ (UNC) + { + vsb.Append(@"UNC\"); + vsb.Append(fullPath.Slice(2)); + } + } + else if (fullPath.Length >= 4 && fullPath[0] == '\\' && fullPath[1] == '?' && fullPath[2] == '?' && fullPath[3] == '\\') // \??\ + { + vsb.Append(fullPath.Slice(NtPrefix.Length)); + } + else + { + vsb.Append(fullPath); + } + } } } From ed9841c96d6caa151a63894db2e3df4c64d9cbef Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 22 Jun 2021 10:43:31 +0200 Subject: [PATCH 2/2] stop using NtCreateFile as there is no public and reliable way of mapping DOS to NT paths --- .../Kernel32/Interop.FILE_ALLOCATION_INFO.cs | 16 +++ .../Common/tests/Common.Tests.csproj | 2 - .../System/IO/PathInternal.Windows.Tests.cs | 43 ------ .../SafeHandles/SafeFileHandle.Windows.cs | 128 +++++++++++++----- .../System.Private.CoreLib.Shared.projitems | 6 + .../src/System/IO/PathInternal.Windows.cs | 30 ---- 6 files changed, 116 insertions(+), 109 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs diff --git a/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs new file mode 100644 index 0000000000000..0233626ee73c6 --- /dev/null +++ b/src/libraries/Common/src/Interop/Windows/Kernel32/Interop.FILE_ALLOCATION_INFO.cs @@ -0,0 +1,16 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +internal static partial class Interop +{ + internal static partial class Kernel32 + { + // Value taken from https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfileinformationbyhandle#remarks: + internal const int FileAllocationInfo = 5; + + internal struct FILE_ALLOCATION_INFO + { + internal long AllocationSize; + } + } +} diff --git a/src/libraries/Common/tests/Common.Tests.csproj b/src/libraries/Common/tests/Common.Tests.csproj index cbbf17ff19005..ba76497cfc556 100644 --- a/src/libraries/Common/tests/Common.Tests.csproj +++ b/src/libraries/Common/tests/Common.Tests.csproj @@ -109,8 +109,6 @@ - DosToNtPathTest_Data => new TheoryData - { - { @"C:\tests\file.cs", @"\??\C:\tests\file.cs" }, // typical path - { @"\\?\C:\tests\file.cs", @"\??\C:\tests\file.cs" }, // NtPath with \\?\ prefix - { @"\\.\device\file.cs", @"\??\device\file.cs" }, // device path with \\.\ prefix - { @"\\server\file.cs", @"\??\UNC\server\file.cs" }, // UNC path with \\ prefix - { @"\\?\UNC\server\file", @"\??\UNC\server\file" }, // extended UNC prefix - { @"\??\C:\tests\file.cs", @"\??\C:\tests\file.cs" }, // NtPath with \??\ prefix (no changes required) - { @"C:\", @"\??\C:\" }, // a short path - { @"\\s", @"\??\UNC\s" }, // short UNC path - { @"\\s\", @"\??\UNC\s\" }, // short UNC path with trailing - { $@"C:\{string.Join("\\", Enumerable.Repeat("a", PathInternal.MaxShortPath + 1))}", $@"\??\C:\{string.Join("\\", Enumerable.Repeat("a", PathInternal.MaxShortPath + 1))}"}, // long path - }; - - [Theory, MemberData(nameof(DosToNtPathTest_Data))] - public void DosToNtPathTest(string path, string expected) - { - // first of all, we use an internal Windows API to ensure that expected value is valid - if (path.Length < PathInternal.MaxShortPath) // RtlDosPathNameToRelativeNtPathName_U_WithStatus does not support long paths - { - RtlDosPathNameToRelativeNtPathName_U_WithStatus(path, out Interop.UNICODE_STRING ntFileName, out IntPtr _, IntPtr.Zero); - try - { - Assert.Equal(expected, Marshal.PtrToStringUni(ntFileName.Buffer)); - } - finally - { - Marshal.ZeroFreeGlobalAllocUnicode(ntFileName.Buffer); - } - } - - // after that, we test our implementation - var vsb = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]); - PathInternal.DosToNtPath(path, ref vsb); - Assert.Equal(expected, vsb.ToString()); - - [DllImport(Interop.Libraries.NtDll, CharSet = CharSet.Unicode)] - static extern int RtlDosPathNameToRelativeNtPathName_U_WithStatus(string DosFileName, out Interop.UNICODE_STRING NtFileName, out IntPtr FilePart, IntPtr RelativeName); - } } } diff --git a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs index 67b239d2aa0ce..5aa7a2c7f4dd6 100644 --- a/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Windows.cs @@ -4,7 +4,8 @@ using System; using System.Diagnostics; using System.IO; -using System.Text; +using System.IO.Strategies; +using System.Runtime.InteropServices; using System.Threading; namespace Microsoft.Win32.SafeHandles @@ -24,13 +25,6 @@ public SafeFileHandle(IntPtr preexistingHandle, bool ownsHandle) : base(ownsHand SetHandle(preexistingHandle); } - private SafeFileHandle(IntPtr preexistingHandle, bool ownsHandle, FileOptions fileOptions) : base(ownsHandle) - { - SetHandle(preexistingHandle); - - _fileOptions = fileOptions; - } - public bool IsAsync => (GetFileOptions() & FileOptions.Asynchronous) != 0; internal bool CanSeek => !IsClosed && GetFileType() == Interop.Kernel32.FileTypes.FILE_TYPE_DISK; @@ -43,10 +37,14 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA { using (DisableMediaInsertionPrompt.Create()) { - SafeFileHandle fileHandle = new SafeFileHandle( - NtCreateFile(fullPath, mode, access, share, options, preallocationSize), - ownsHandle: true, - options); + // we don't use NtCreateFile as there is no public and reliable way + // of converting DOS to NT file paths (RtlDosPathNameToRelativeNtPathName_U_WithStatus is not documented) + SafeFileHandle fileHandle = CreateFile(fullPath, mode, access, share, options); + + if (FileStreamHelpers.ShouldPreallocate(preallocationSize, access, mode)) + { + Preallocate(fullPath, preallocationSize, fileHandle); + } fileHandle.InitThreadPoolBindingIfNeeded(); @@ -54,29 +52,91 @@ internal static unsafe SafeFileHandle Open(string fullPath, FileMode mode, FileA } } - private static IntPtr NtCreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize) + private static unsafe SafeFileHandle CreateFile(string fullPath, FileMode mode, FileAccess access, FileShare share, FileOptions options) + { + Interop.Kernel32.SECURITY_ATTRIBUTES secAttrs = default; + if ((share & FileShare.Inheritable) != 0) + { + secAttrs = new Interop.Kernel32.SECURITY_ATTRIBUTES + { + nLength = (uint)sizeof(Interop.Kernel32.SECURITY_ATTRIBUTES), + bInheritHandle = Interop.BOOL.TRUE + }; + } + + int fAccess = + ((access & FileAccess.Read) == FileAccess.Read ? Interop.Kernel32.GenericOperations.GENERIC_READ : 0) | + ((access & FileAccess.Write) == FileAccess.Write ? Interop.Kernel32.GenericOperations.GENERIC_WRITE : 0); + + // Our Inheritable bit was stolen from Windows, but should be set in + // the security attributes class. Don't leave this bit set. + share &= ~FileShare.Inheritable; + + // Must use a valid Win32 constant here... + if (mode == FileMode.Append) + { + mode = FileMode.OpenOrCreate; + } + + int flagsAndAttributes = (int)options; + + // For mitigating local elevation of privilege attack through named pipes + // make sure we always call CreateFile with SECURITY_ANONYMOUS so that the + // named pipe server can't impersonate a high privileged client security context + // (note that this is the effective default on CreateFile2) + flagsAndAttributes |= (Interop.Kernel32.SecurityOptions.SECURITY_SQOS_PRESENT | Interop.Kernel32.SecurityOptions.SECURITY_ANONYMOUS); + + SafeFileHandle fileHandle = Interop.Kernel32.CreateFile(fullPath, fAccess, share, &secAttrs, mode, flagsAndAttributes, IntPtr.Zero); + if (fileHandle.IsInvalid) + { + // Return a meaningful exception with the full path. + + // NT5 oddity - when trying to open "C:\" as a Win32FileStream, + // we usually get ERROR_PATH_NOT_FOUND from the OS. We should + // probably be consistent w/ every other directory. + int errorCode = Marshal.GetLastPInvokeError(); + + if (errorCode == Interop.Errors.ERROR_PATH_NOT_FOUND && fullPath!.Length == PathInternal.GetRootLength(fullPath)) + { + errorCode = Interop.Errors.ERROR_ACCESS_DENIED; + } + + throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); + } + + fileHandle._fileOptions = options; + return fileHandle; + } + + private static unsafe void Preallocate(string fullPath, long preallocationSize, SafeFileHandle fileHandle) { - var vsb = new ValueStringBuilder(stackalloc char[PathInternal.MaxShortPath]); - - PathInternal.DosToNtPath(fullPath, ref vsb); - - (uint ntStatus, IntPtr fileHandle) = Interop.NtDll.NtCreateFile(vsb.AsSpan(), mode, access, share, options, preallocationSize); - vsb.Dispose(); - - switch (ntStatus) - { - case Interop.StatusOptions.STATUS_SUCCESS: - return fileHandle; - case Interop.StatusOptions.STATUS_DISK_FULL: - throw new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, fullPath, preallocationSize)); - // NtCreateFile has a bug and it reports STATUS_INVALID_PARAMETER for files - // that are too big for the current file system. Example: creating a 4GB+1 file on a FAT32 drive. - case Interop.StatusOptions.STATUS_INVALID_PARAMETER when preallocationSize > 0: - case Interop.StatusOptions.STATUS_FILE_TOO_LARGE: - throw new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, fullPath, preallocationSize)); - default: - int error = (int)Interop.NtDll.RtlNtStatusToDosError((int)ntStatus); - throw Win32Marshal.GetExceptionForWin32Error(error, fullPath); + var allocationInfo = new Interop.Kernel32.FILE_ALLOCATION_INFO + { + AllocationSize = preallocationSize + }; + + if (!Interop.Kernel32.SetFileInformationByHandle( + fileHandle, + Interop.Kernel32.FileAllocationInfo, + &allocationInfo, + (uint)sizeof(Interop.Kernel32.FILE_ALLOCATION_INFO))) + { + int errorCode = Marshal.GetLastPInvokeError(); + + // we try to mimic the atomic NtCreateFile here: + // if preallocation fails, close the handle and delete the file + fileHandle.Dispose(); + Interop.Kernel32.DeleteFile(fullPath); + + switch (errorCode) + { + case Interop.Errors.ERROR_DISK_FULL: + throw new IOException(SR.Format(SR.IO_DiskFull_Path_AllocationSize, fullPath, preallocationSize)); + case Interop.Errors.ERROR_FILE_TOO_LARGE: + throw new IOException(SR.Format(SR.IO_FileTooLarge_Path_AllocationSize, fullPath, preallocationSize)); + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath); + } } } diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index a189be9ac1517..1f457e0b5a312 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1425,6 +1425,9 @@ Common\Interop\Windows\Kernel32\Interop.FILE_BASIC_INFO.cs + + Common\Interop\Windows\Kernel32\Interop.FILE_ALLOCATION_INFO.cs + Common\Interop\Windows\Kernel32\Interop.FILE_END_OF_FILE_INFO.cs @@ -1611,6 +1614,9 @@ Common\Interop\Windows\Interop.UNICODE_STRING.cs + + Common\Interop\Windows\Kernel32\Interop.SecurityOptions.cs + Common\Interop\Windows\Interop.SECURITY_QUALITY_OF_SERVICE.cs diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs index 3ee5bcdab9a29..dd9442b165bce 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs @@ -47,7 +47,6 @@ internal static partial class PathInternal internal const string DirectorySeparatorCharAsString = "\\"; internal const string ExtendedPathPrefix = @"\\?\"; - internal const string NtPrefix = @"\??\"; internal const string UncPathPrefix = @"\\"; internal const string UncExtendedPrefixToInsert = @"?\UNC\"; internal const string UncExtendedPathPrefix = @"\\?\UNC\"; @@ -410,34 +409,5 @@ internal static bool IsEffectivelyEmpty(ReadOnlySpan path) } return true; } - - // this method works only for `fullPath` returned by Path.GetFullPath - // currently we don't have interest in supporting relative paths - internal static void DosToNtPath(ReadOnlySpan fullPath, ref ValueStringBuilder vsb) - { - vsb.Append(NtPrefix); - - if (fullPath.Length >= 3 && fullPath[0] == '\\' && fullPath[1] == '\\') - { - // \\.\ (Device) or \\?\ (NtPath) - if (fullPath.Length >= 4 && fullPath[3] == '\\' && (fullPath[2] == '.' || fullPath[2] == '?')) - { - vsb.Append(fullPath.Slice(NtPrefix.Length)); - } - else // \\ (UNC) - { - vsb.Append(@"UNC\"); - vsb.Append(fullPath.Slice(2)); - } - } - else if (fullPath.Length >= 4 && fullPath[0] == '\\' && fullPath[1] == '?' && fullPath[2] == '?' && fullPath[3] == '\\') // \??\ - { - vsb.Append(fullPath.Slice(NtPrefix.Length)); - } - else - { - vsb.Append(fullPath); - } - } } }