diff --git a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs index cc8ba755199efd..e1ef1f71236bc9 100644 --- a/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs +++ b/src/libraries/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs @@ -90,16 +90,30 @@ public PEHeaders(Stream peStream, int size, bool isLoadedImage) SkipDosHeader(ref reader, out bool isCoffOnly); + if (isCoffOnly && isLoadedImage) + { + // Only images can be loaded. + throw new BadImageFormatException(SR.InvalidPESignature); + } + _coffHeaderStartOffset = reader.Offset; _coffHeader = new CoffHeader(ref reader); - if (!isCoffOnly) + if (isCoffOnly) + { + // In COFF files the size of the optional header must be zero. + if (_coffHeader.SizeOfOptionalHeader != 0) + { + throw new BadImageFormatException(SR.UnknownFileFormat); + } + } + else { _peHeaderStartOffset = reader.Offset; _peHeader = new PEHeader(ref reader); } - _sectionHeaders = ReadSectionHeaders(ref reader); + _sectionHeaders = ReadSectionHeaders(ref reader, actualSize); if (!isCoffOnly) { @@ -289,10 +303,10 @@ private static void SkipDosHeader(ref PEBinaryReader reader, out bool isCOFFOnly } } - private ImmutableArray ReadSectionHeaders(ref PEBinaryReader reader) + private ImmutableArray ReadSectionHeaders(ref PEBinaryReader reader, int size) { int numberOfSections = _coffHeader.NumberOfSections; - if (numberOfSections < 0) + if (numberOfSections < 0 || numberOfSections * SectionHeader.Size > size - reader.Offset) { throw new BadImageFormatException(SR.InvalidNumberOfSections); } @@ -384,16 +398,8 @@ private void CalculateMetadataLocation(long peImageSize, out int start, out int return; } - if (_isLoadedImage) - { - start = SectionHeaders[cormeta].VirtualAddress; - size = SectionHeaders[cormeta].VirtualSize; - } - else - { - start = SectionHeaders[cormeta].PointerToRawData; - size = SectionHeaders[cormeta].SizeOfRawData; - } + start = SectionHeaders[cormeta].PointerToRawData; + size = SectionHeaders[cormeta].SizeOfRawData; } else if (_corHeader == null) { diff --git a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEHeadersTests.cs b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEHeadersTests.cs index a5be92ff0d2f20..e38dae2bf59e44 100644 --- a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEHeadersTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEHeadersTests.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers.Binary; using System.IO; using System.Linq; using System.Reflection.Metadata.Tests; @@ -50,6 +51,29 @@ public void Ctor_Streams() Assert.Equal(0, s.Position); } + [Fact] + public void Ctor_InvalidCoffHeader() + { + byte[] header = new byte[CoffHeader.Size]; + _ = new PEHeaders(new MemoryStream(header)); + + // NumberOfSections is too big. + BinaryPrimitives.WriteInt16LittleEndian(header.AsSpan(16), 5); + Assert.Throws(() => new PEHeaders(new MemoryStream(header))); + + header = new byte[CoffHeader.Size]; + // SizeOfOptionalHeader is non-zero. + BinaryPrimitives.WriteInt16LittleEndian(header.AsSpan(2), 10); + Assert.Throws(() => new PEHeaders(new MemoryStream(header))); + } + + [Fact] + public void Ctor_CoffFileLoadedImage() + { + byte[] file = new byte[CoffHeader.Size]; + Assert.Throws(() => new PEHeaders(new MemoryStream(file), file.Length, isLoadedImage: true)); + } + [Fact] public void FromEmptyStream() { diff --git a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs index b367fff3c382c5..13c9deab4498e7 100644 --- a/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs +++ b/src/libraries/System.Reflection.Metadata/tests/PortableExecutable/PEReaderTests.cs @@ -820,10 +820,7 @@ public void Dispose() Assert.Throws(() => reader.ReadDebugDirectory()); Assert.Throws(() => reader.ReadCodeViewDebugDirectoryData(ddCodeView)); Assert.Throws(() => reader.ReadEmbeddedPortablePdbDebugDirectoryData(ddEmbedded)); - - MetadataReaderProvider __; - string ___; - Assert.Throws(() => reader.TryOpenAssociatedPortablePdb(@"x", _ => null, out __, out ___)); + Assert.Throws(() => reader.TryOpenAssociatedPortablePdb(@"x", _ => null, out _, out _)); // ok to use providers after PEReader disposed: var pdbReader = pdbProvider.GetMetadataReader(); @@ -869,15 +866,5 @@ public unsafe void InvokeCtorWithIsLoadedImageAndPrefetchMetadataOptions2() } } } - - [Fact] - public void HasMetadataShouldReturnFalseWhenPrefetchingMetadataOfImageWithoutMetadata() - { - using (var fileStream = new MemoryStream(Misc.KeyPair)) - using (var peReader = new PEReader(fileStream, PEStreamOptions.PrefetchMetadata | PEStreamOptions.LeaveOpen)) - { - Assert.False(peReader.HasMetadata); - } - } } }