dotnet / runtime

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

System.Reflection.Metadata fails to read offset of DataDirectories spanning across multiple sections #14556

Closed krwq closed 4 years ago

krwq commented 9 years ago

If an assembly contains any DataDirectory which is in more than one section we throw "System.BadImageFormatException : Section too small.". While I agree we should not have any assemblies like that we unfortunately already shipped one which has this issue.

I believe we can still calculate the offset without checking ranges (or we can check the global range only)

Example can be found here: https://github.com/krwq/corefx/tree/pereaderdatadir

This is basically:

using (FileStream fs = new FileStream(@"C:\Windows\Microsoft.NET\Framework\v4.0.30319\Microsoft.JScript.dll", FileMode.Open, FileAccess.Read, FileShare.ReadWrite | FileShare.Delete))
{
    using (PEReader reader = new PEReader(fs, PEStreamOptions.LeaveOpen | PEStreamOptions.PrefetchEntireImage | PEStreamOptions.PrefetchMetadata))
    {
        int offset;
        if (reader.PEHeaders.TryGetDirectoryOffset(reader.PEHeaders.PEHeader.CertificateTableDirectory, out offset))
        {

        }
    }
}
nguerrera commented 9 years ago

cc @tmat

krwq commented 9 years ago

I believe that removing those lines should fix this bug: https://github.com/dotnet/corefx/blob/master/src/System.Reflection.Metadata/src/System/Reflection/PortableExecutable/PEHeaders.cs#L307-L310

I believe those 4 lines are invalid as we are just trying to find the offset relative to beginning of file and not in which section that directory is so information if it fits the section is irrelevant.

@nguerrera, @tmat could you verify that removing it is ok?