dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.04k stars 4.68k forks source link

PEReader doesn't work with "IsLoadedImage" and "PrefetchMetadata" options #76648

Closed andrewcrawley closed 1 year ago

andrewcrawley commented 1 year ago

Description

I'm creating a PEReader to pull some metadata out of an assembly that's already loaded in memory. If I pass the IsLoadedImage and PrefetchMetadata flags together, PEReader throws a BadImageFormatException ("Missing data directory.").

Other combinations of flags (e.g. IsLoadedImage alone, or IsLoadedImage + PrefetchEntireImage) work fine.

Reproduction Steps

Paste the following into a .net 6 Windows console app and run.

using System.Diagnostics;
using System.Reflection.PortableExecutable;
using System.Runtime.InteropServices;

namespace ConsoleApp31
{
    internal class Program
    {
        [DllImport("kernel32.dll")]
        public static extern bool ReadProcessMemory(
            IntPtr hProcess,
            UInt64 lpBaseAddress,
            byte[] lpBuffer,
            int nSize,
            ref int lpNumberOfBytesRead);

        private const int PageSize = 4096;

        static void Main(string[] args)
        {
            Process currentProc = Process.GetCurrentProcess();

            string procName = Path.GetFileName(currentProc.ProcessName);
            string libName = Path.ChangeExtension(procName, "dll");

            ProcessModule assemblyModule = currentProc.Modules.OfType<ProcessModule>().FirstOrDefault(m => String.Equals(m.ModuleName, libName, StringComparison.OrdinalIgnoreCase));

            UInt64 baseAddress = (UInt64)assemblyModule.BaseAddress.ToInt64();
            int length = assemblyModule.ModuleMemorySize;

            // Read assembly contents out of memory, skipping unmapped pages
            byte[] assemblyBytes = new byte[length];
            byte[] page = new byte[PageSize];
            int bytesRead = 0;
            for (uint i=0; i < length; i+= PageSize)
            {
                ReadProcessMemory(currentProc.Handle, baseAddress + i, page, PageSize, ref bytesRead);
                Array.Copy(page, 0, assemblyBytes, i, bytesRead);
            }

            MemoryStream assemblyStream = new MemoryStream(assemblyBytes);

            // Succeeds
            PEReader prefetchNothingReader = new PEReader(assemblyStream, PEStreamOptions.IsLoadedImage | PEStreamOptions.LeaveOpen);

            // Succeeds
            assemblyStream.Seek(0, SeekOrigin.Begin);
            PEReader prefetchEntireImageReader = new PEReader(assemblyStream, PEStreamOptions.IsLoadedImage | PEStreamOptions.PrefetchEntireImage | PEStreamOptions.LeaveOpen);

            // Throws BadImageFormatException: Missing data directory.
            assemblyStream.Seek(0, SeekOrigin.Begin);
            PEReader prefetchMetadataReader = new PEReader(assemblyStream, PEStreamOptions.IsLoadedImage | PEStreamOptions.PrefetchMetadata | PEStreamOptions.LeaveOpen);
        }
    }
}

Expected behavior

Passing the IsLoadedImage and PrefetchMetadata flags should result in a usable PEReader.

Actual behavior

Passing the IsLoadedImage and PrefetchMetadata flags results in an exception being thrown:

System.BadImageFormatException
  HResult=0x8007000B
  Message=Missing data directory.
  Source=System.Reflection.Metadata
  StackTrace:
   at System.Reflection.PortableExecutable.PEHeaders.CalculateMetadataLocation(Int64 peImageSize, Int32& start, Int32& size)
   at System.Reflection.PortableExecutable.PEHeaders..ctor(Stream peStream, Int32 size, Boolean isLoadedImage)
   at System.Reflection.PortableExecutable.PEHeaders..ctor(Stream peStream, Int32 size)
   at System.Reflection.PortableExecutable.PEHeaders..ctor(Stream peStream)
   at System.Reflection.PortableExecutable.PEReader..ctor(Stream peStream, PEStreamOptions options, Int32 size)
   at System.Reflection.PortableExecutable.PEReader..ctor(Stream peStream, PEStreamOptions options)
   at ConsoleApp31.Program.Main(String[] args)

Regression?

Not a regression.

Known Workarounds

I can avoid passing the IsLoadedImage and PrefetchMetadata flags together, but I'd prefer to be able to prefetch the metadata so I no longer need to keep the stream around. PrefetchEntireImage works, but spends time / memory reading a bunch of extra data I don't need.

Configuration

Provided repro uses .net 6 on Windows AMD64, but the issue isn't specific to that configuration.

Other information

It appears that the IsLoadedImage flag doesn't get propagated to the PEHeaders object that is created in the PEReader constructor for this combination of flags, resulting in invalid RVAs being calculated for various headers. See: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.cs#L201

In local testing, the issue no longer repros for me after patching PEReader to pass the flag along.

dotnet-issue-labeler[bot] commented 1 year ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-reflection-metadata See info in area-owners.md if you want to be subscribed.

Issue Details
### Description I'm creating a `PEReader` to pull some metadata out of an assembly that's already loaded in memory. If I pass the `IsLoadedImage` and `PrefetchMetadata` flags together, `PEReader` throws a `BadImageFormatException` ("Missing data directory."). Other combinations of flags (e.g. `IsLoadedImage` alone, or `IsLoadedImage` + `PrefetchEntireImage`) work fine. ### Reproduction Steps Paste the following into a .net 6 Windows console app and run. ```C# using System.Diagnostics; using System.Reflection.PortableExecutable; using System.Runtime.InteropServices; namespace ConsoleApp31 { internal class Program { [DllImport("kernel32.dll")] public static extern bool ReadProcessMemory( IntPtr hProcess, UInt64 lpBaseAddress, byte[] lpBuffer, int nSize, ref int lpNumberOfBytesRead); private const int PageSize = 4096; static void Main(string[] args) { Process currentProc = Process.GetCurrentProcess(); string procName = Path.GetFileName(currentProc.ProcessName); string libName = Path.ChangeExtension(procName, "dll"); ProcessModule assemblyModule = currentProc.Modules.OfType().FirstOrDefault(m => String.Equals(m.ModuleName, libName, StringComparison.OrdinalIgnoreCase)); UInt64 baseAddress = (UInt64)assemblyModule.BaseAddress.ToInt64(); int length = assemblyModule.ModuleMemorySize; // Read assembly contents out of memory, skipping unmapped pages byte[] assemblyBytes = new byte[length]; byte[] page = new byte[PageSize]; int bytesRead = 0; for (uint i=0; i < length; i+= PageSize) { ReadProcessMemory(currentProc.Handle, baseAddress + i, page, PageSize, ref bytesRead); Array.Copy(page, 0, assemblyBytes, i, bytesRead); } MemoryStream assemblyStream = new MemoryStream(assemblyBytes); // Succeeds PEReader prefetchNothingReader = new PEReader(assemblyStream, PEStreamOptions.IsLoadedImage | PEStreamOptions.LeaveOpen); // Succeeds assemblyStream.Seek(0, SeekOrigin.Begin); PEReader prefetchEntireImageReader = new PEReader(assemblyStream, PEStreamOptions.IsLoadedImage | PEStreamOptions.PrefetchEntireImage | PEStreamOptions.LeaveOpen); // Throws BadImageFormatException: Missing data directory. assemblyStream.Seek(0, SeekOrigin.Begin); PEReader prefetchMetadataReader = new PEReader(assemblyStream, PEStreamOptions.IsLoadedImage | PEStreamOptions.PrefetchMetadata | PEStreamOptions.LeaveOpen); } } } ``` ### Expected behavior Passing the `IsLoadedImage` and `PrefetchMetadata` flags should result in a usable `PEReader`. ### Actual behavior Passing the `IsLoadedImage` and `PrefetchMetadata` flags results in an exception being thrown: ``` System.BadImageFormatException HResult=0x8007000B Message=Missing data directory. Source=System.Reflection.Metadata StackTrace: at System.Reflection.PortableExecutable.PEHeaders.CalculateMetadataLocation(Int64 peImageSize, Int32& start, Int32& size) at System.Reflection.PortableExecutable.PEHeaders..ctor(Stream peStream, Int32 size, Boolean isLoadedImage) at System.Reflection.PortableExecutable.PEHeaders..ctor(Stream peStream, Int32 size) at System.Reflection.PortableExecutable.PEHeaders..ctor(Stream peStream) at System.Reflection.PortableExecutable.PEReader..ctor(Stream peStream, PEStreamOptions options, Int32 size) at System.Reflection.PortableExecutable.PEReader..ctor(Stream peStream, PEStreamOptions options) at ConsoleApp31.Program.Main(String[] args) ``` ### Regression? Not a regression. ### Known Workarounds I can avoid passing the `IsLoadedImage` and `PrefetchMetadata` flags together, but I'd prefer to be able to prefetch the metadata so I no longer need to keep the stream around. `PrefetchEntireImage` works, but spends time / memory reading a bunch of extra data I don't need. ### Configuration Provided repro uses .net 6 on Windows AMD64, but the issue isn't specific to that configuration. ### Other information It appears that the `IsLoadedImage` flag doesn't get propagated to the `PEHeaders` object that is created in the `PEReader` constructor for this combination of flags, resulting in invalid RVAs being calculated for various headers. See: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEReader.cs#L201 In local testing, the issue no longer repros for me after patching `PEReader` to pass the flag along.
Author: andrewcrawley
Assignees: -
Labels: `area-System.Reflection.Metadata`, `untriaged`
Milestone: -
buyaa-n commented 1 year ago

Thanks for reporting the issue @andrewcrawley, any interest in fixing the bug? Feel free to raise a PR with corresponding unit tests(s)