From 0566e7dd9a7a8e5dec119d0ba715623beeba4c90 Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Wed, 20 Sep 2023 17:27:40 -0700 Subject: [PATCH 1/2] Make HostModel PEUtils always read/write little endian --- .../AppHost/PEUtils.cs | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs b/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs index 0d0b33ed55569..e6142aba26e3e 100644 --- a/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs +++ b/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; +using System.Buffers.Binary; using System.IO; using System.IO.MemoryMappedFiles; @@ -24,7 +26,9 @@ internal static unsafe bool IsPEImage(MemoryMappedViewAccessor accessor) // https://en.wikipedia.org/wiki/Portable_Executable // Validate that we're looking at Windows PE file - if (((ushort*)bytes)[0] != PEOffsets.DosImageSignature + ReadOnlySpan signatureBytes = new(bytes, sizeof(ushort)); + ushort signature = BinaryPrimitives.ReadUInt16LittleEndian(signatureBytes); + if (signature != PEOffsets.DosImageSignature || accessor.Capacity < PEOffsets.DosStub.PESignatureOffset + sizeof(uint)) { return false; @@ -68,24 +72,26 @@ internal static unsafe void SetWindowsGraphicalUserInterfaceBit(MemoryMappedView byte* bytes = pointer + accessor.PointerOffset; // https://en.wikipedia.org/wiki/Portable_Executable - uint peHeaderOffset = ((uint*)(bytes + PEOffsets.DosStub.PESignatureOffset))[0]; + ReadOnlySpan peHeaderOffsetBytes = new(bytes + PEOffsets.DosStub.PESignatureOffset, sizeof(uint)); + uint peHeaderOffset = BinaryPrimitives.ReadUInt32LittleEndian(peHeaderOffsetBytes); if (accessor.Capacity < peHeaderOffset + PEOffsets.PEHeader.Subsystem + sizeof(ushort)) { throw new AppHostNotPEFileException("Subsystem offset out of file range."); } - ushort* subsystem = ((ushort*)(bytes + peHeaderOffset + PEOffsets.PEHeader.Subsystem)); + Span subsystemBytes = new(bytes + peHeaderOffset + PEOffsets.PEHeader.Subsystem, sizeof(ushort)); + ushort subsystem = BinaryPrimitives.ReadUInt16LittleEndian(subsystemBytes); // https://docs.microsoft.com/en-us/windows/desktop/Debug/pe-format#windows-subsystem // The subsystem of the prebuilt apphost should be set to CUI - if (subsystem[0] != (ushort)PEOffsets.Subsystem.WindowsCui) + if (subsystem != (ushort)PEOffsets.Subsystem.WindowsCui) { - throw new AppHostNotCUIException(subsystem[0]); + throw new AppHostNotCUIException(subsystem); } // Set the subsystem to GUI - subsystem[0] = (ushort)PEOffsets.Subsystem.WindowsGui; + BinaryPrimitives.WriteUInt16LittleEndian(subsystemBytes, (ushort)PEOffsets.Subsystem.WindowsGui); } finally { @@ -121,16 +127,16 @@ internal static unsafe ushort GetWindowsGraphicalUserInterfaceBit(MemoryMappedVi byte* bytes = pointer + accessor.PointerOffset; // https://en.wikipedia.org/wiki/Portable_Executable - uint peHeaderOffset = ((uint*)(bytes + PEOffsets.DosStub.PESignatureOffset))[0]; + ReadOnlySpan peHeaderOffsetBytes = new(bytes + PEOffsets.DosStub.PESignatureOffset, sizeof(uint)); + uint peHeaderOffset = BinaryPrimitives.ReadUInt32LittleEndian(peHeaderOffsetBytes); if (accessor.Capacity < peHeaderOffset + PEOffsets.PEHeader.Subsystem + sizeof(ushort)) { throw new AppHostNotPEFileException("Subsystem offset out of file range."); } - ushort* subsystem = ((ushort*)(bytes + peHeaderOffset + PEOffsets.PEHeader.Subsystem)); - - return subsystem[0]; + ReadOnlySpan subsystemBytes = new(bytes + peHeaderOffset + PEOffsets.PEHeader.Subsystem, sizeof(ushort)); + return BinaryPrimitives.ReadUInt16LittleEndian(subsystemBytes); } finally { From d39f18ecd6671748cc6baf57f141f94668cb5f9e Mon Sep 17 00:00:00 2001 From: Elinor Fung Date: Thu, 21 Sep 2023 10:45:36 -0700 Subject: [PATCH 2/2] PR feeback - helper methods --- .../AppHost/PEUtils.cs | 123 ++++++------------ .../AppHostUpdateTests.cs | 6 +- 2 files changed, 42 insertions(+), 87 deletions(-) diff --git a/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs b/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs index e6142aba26e3e..1bfc80fcfca49 100644 --- a/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs +++ b/src/runtime/src/installer/managed/Microsoft.NET.HostModel/AppHost/PEUtils.cs @@ -5,6 +5,7 @@ using System.Buffers.Binary; using System.IO; using System.IO.MemoryMappedFiles; +using System.Reflection.PortableExecutable; namespace Microsoft.NET.HostModel.AppHost { @@ -17,31 +18,13 @@ public static class PEUtils /// true if the accessor represents a PE image, false otherwise. internal static unsafe bool IsPEImage(MemoryMappedViewAccessor accessor) { - byte* pointer = null; + if (accessor.Capacity < PEOffsets.DosStub.PESignatureOffset + sizeof(uint)) + return false; - try - { - accessor.SafeMemoryMappedViewHandle.AcquirePointer(ref pointer); - byte* bytes = pointer + accessor.PointerOffset; - - // https://en.wikipedia.org/wiki/Portable_Executable - // Validate that we're looking at Windows PE file - ReadOnlySpan signatureBytes = new(bytes, sizeof(ushort)); - ushort signature = BinaryPrimitives.ReadUInt16LittleEndian(signatureBytes); - if (signature != PEOffsets.DosImageSignature - || accessor.Capacity < PEOffsets.DosStub.PESignatureOffset + sizeof(uint)) - { - return false; - } - return true; - } - finally - { - if (pointer != null) - { - accessor.SafeMemoryMappedViewHandle.ReleasePointer(); - } - } + // https://en.wikipedia.org/wiki/Portable_Executable + // Validate that we're looking at Windows PE file + ushort signature = AsLittleEndian(accessor.ReadUInt16(0)); + return signature == PEOffsets.DosImageSignature; } public static bool IsPEImage(string filePath) @@ -64,42 +47,15 @@ public static bool IsPEImage(string filePath) /// The memory accessor which has the apphost file opened. internal static unsafe void SetWindowsGraphicalUserInterfaceBit(MemoryMappedViewAccessor accessor) { - byte* pointer = null; - - try - { - accessor.SafeMemoryMappedViewHandle.AcquirePointer(ref pointer); - byte* bytes = pointer + accessor.PointerOffset; - - // https://en.wikipedia.org/wiki/Portable_Executable - ReadOnlySpan peHeaderOffsetBytes = new(bytes + PEOffsets.DosStub.PESignatureOffset, sizeof(uint)); - uint peHeaderOffset = BinaryPrimitives.ReadUInt32LittleEndian(peHeaderOffsetBytes); - - if (accessor.Capacity < peHeaderOffset + PEOffsets.PEHeader.Subsystem + sizeof(ushort)) - { - throw new AppHostNotPEFileException("Subsystem offset out of file range."); - } - - Span subsystemBytes = new(bytes + peHeaderOffset + PEOffsets.PEHeader.Subsystem, sizeof(ushort)); - ushort subsystem = BinaryPrimitives.ReadUInt16LittleEndian(subsystemBytes); - - // https://docs.microsoft.com/en-us/windows/desktop/Debug/pe-format#windows-subsystem - // The subsystem of the prebuilt apphost should be set to CUI - if (subsystem != (ushort)PEOffsets.Subsystem.WindowsCui) - { - throw new AppHostNotCUIException(subsystem); - } - - // Set the subsystem to GUI - BinaryPrimitives.WriteUInt16LittleEndian(subsystemBytes, (ushort)PEOffsets.Subsystem.WindowsGui); - } - finally - { - if (pointer != null) - { - accessor.SafeMemoryMappedViewHandle.ReleasePointer(); - } - } + // https://learn.microsoft.com/windows/win32/debug/pe-format#windows-subsystem + // The subsystem of the prebuilt apphost should be set to CUI + uint peHeaderOffset; + ushort subsystem = GetWindowsSubsystem(accessor, out peHeaderOffset); + if (subsystem != (ushort)Subsystem.WindowsCui) + throw new AppHostNotCUIException(subsystem); + + // Set the subsystem to GUI + accessor.Write(peHeaderOffset + PEOffsets.PEHeader.Subsystem, AsLittleEndian((ushort)Subsystem.WindowsGui)); } public static unsafe void SetWindowsGraphicalUserInterfaceBit(string filePath) @@ -119,32 +75,7 @@ public static unsafe void SetWindowsGraphicalUserInterfaceBit(string filePath) /// The memory accessor which has the apphost file opened. internal static unsafe ushort GetWindowsGraphicalUserInterfaceBit(MemoryMappedViewAccessor accessor) { - byte* pointer = null; - - try - { - accessor.SafeMemoryMappedViewHandle.AcquirePointer(ref pointer); - byte* bytes = pointer + accessor.PointerOffset; - - // https://en.wikipedia.org/wiki/Portable_Executable - ReadOnlySpan peHeaderOffsetBytes = new(bytes + PEOffsets.DosStub.PESignatureOffset, sizeof(uint)); - uint peHeaderOffset = BinaryPrimitives.ReadUInt32LittleEndian(peHeaderOffsetBytes); - - if (accessor.Capacity < peHeaderOffset + PEOffsets.PEHeader.Subsystem + sizeof(ushort)) - { - throw new AppHostNotPEFileException("Subsystem offset out of file range."); - } - - ReadOnlySpan subsystemBytes = new(bytes + peHeaderOffset + PEOffsets.PEHeader.Subsystem, sizeof(ushort)); - return BinaryPrimitives.ReadUInt16LittleEndian(subsystemBytes); - } - finally - { - if (pointer != null) - { - accessor.SafeMemoryMappedViewHandle.ReleasePointer(); - } - } + return GetWindowsSubsystem(accessor, out _); } public static unsafe ushort GetWindowsGraphicalUserInterfaceBit(string filePath) @@ -157,5 +88,25 @@ public static unsafe ushort GetWindowsGraphicalUserInterfaceBit(string filePath) } } } + + private static ushort GetWindowsSubsystem(MemoryMappedViewAccessor accessor, out uint peHeaderOffset) + { + // https://en.wikipedia.org/wiki/Portable_Executable + if (accessor.Capacity < PEOffsets.DosStub.PESignatureOffset + sizeof(uint)) + throw new AppHostNotPEFileException("PESignature offset out of file range."); + + peHeaderOffset = AsLittleEndian(accessor.ReadUInt32(PEOffsets.DosStub.PESignatureOffset)); + if (accessor.Capacity < peHeaderOffset + PEOffsets.PEHeader.Subsystem + sizeof(ushort)) + throw new AppHostNotPEFileException("Subsystem offset out of file range."); + + // https://learn.microsoft.com/windows/win32/debug/pe-format#windows-subsystem + return AsLittleEndian(accessor.ReadUInt16(peHeaderOffset + PEOffsets.PEHeader.Subsystem)); + } + + private static ushort AsLittleEndian(ushort value) + => BitConverter.IsLittleEndian ? value : BinaryPrimitives.ReverseEndianness(value); + + private static uint AsLittleEndian(uint value) + => BitConverter.IsLittleEndian ? value : BinaryPrimitives.ReverseEndianness(value); } } diff --git a/src/runtime/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs b/src/runtime/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs index 232410be8f259..b4d038b99b5ce 100644 --- a/src/runtime/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs +++ b/src/runtime/src/installer/tests/Microsoft.NET.HostModel.Tests/Microsoft.NET.HostModel.AppHost.Tests/AppHostUpdateTests.cs @@ -11,6 +11,7 @@ using Microsoft.NET.HostModel.AppHost; using Microsoft.DotNet.CoreSetup.Test; using System.Diagnostics; +using System.Reflection.PortableExecutable; namespace Microsoft.NET.HostModel.Tests { @@ -111,7 +112,9 @@ public void ItCanSetWindowsGUISubsystem() BitConverter .ToUInt16(File.ReadAllBytes(destinationFilePath), SubsystemOffset) .Should() - .Be(2); + .Be((ushort)Subsystem.WindowsGui); + + Assert.Equal((ushort)Subsystem.WindowsGui, PEUtils.GetWindowsGraphicalUserInterfaceBit(destinationFilePath)); } } @@ -153,6 +156,7 @@ public void ItFailsToSetGUISubsystemWithWrongDefault() string destinationFilePath = Path.Combine(testDirectory.Path, "DestinationAppHost.exe.mock"); string appBinaryFilePath = "Test/App/Binary/Path.dll"; + Assert.Equal(42, PEUtils.GetWindowsGraphicalUserInterfaceBit(sourceAppHostMock)); Assert.Throws(() => HostWriter.CreateAppHost( sourceAppHostMock,