Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PEReader does not throw BadImageFormatException for some invalid PE files #48419

Open
FiniteReality opened this issue Feb 17, 2021 · 3 comments
Labels
area-System.Reflection.Metadata enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@FiniteReality
Copy link

Description

Creating a PEReader for an invalid file type, such as an ELF can sometimes lead to a situation where the PEReader thinks the file is valid and it will return garbage data, causing errors to occur downstream.

Attached is a zipfile containing an ELF which reproduces the issue, trimmed using dd to reduce its filesize while still reproducing the issue.

Here's the code I'm running:

using System.Reflection.PortableExecutable;

var reader = new PEReader(File.OpenRead("path/to/invalid.so"));

Console.WriteLine(reader.HasMetadata);

Running the same code on a different ELF file throws a BadImageFormatException, as expected.

Configuration

.NET SDK/Runtime info
$ dotnet --info
.NET SDK (reflecting any global.json):
 Version:   5.0.102
 Commit:    71365b4d42

Runtime Environment:
 OS Name:     debian
 OS Version:  
 OS Platform: Linux
 RID:         debian-x64
 Base Path:   /usr/share/dotnet/sdk/5.0.102/

Host (useful for support):
  Version: 5.0.2
  Commit:  cb5f173b96

.NET SDKs installed:
  3.1.405 [/usr/share/dotnet/sdk]
  5.0.102 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 3.1.11 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.2 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.11 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.2 [/usr/share/dotnet/shared/Microsoft.NETCore.App]
@dotnet-issue-labeler
Copy link

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.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 17, 2021
@krwq krwq added this to the 7.0.0 milestone Jul 12, 2021
@krwq krwq added the enhancement Product code improvement that does NOT require public API changes/additions label Jul 12, 2021
@krwq krwq removed the untriaged New issue has not been triaged by the area owner label Jul 12, 2021
@buyaa-n buyaa-n modified the milestones: 7.0.0, 8.0.0 Jul 6, 2022
@ericstj ericstj modified the milestones: 8.0.0, 9.0.0 Aug 11, 2023
@ericstj
Copy link
Member

ericstj commented Aug 11, 2023

This is a good suggestion. Maybe we could see how the runtime or linker (or dumpbin) does it and match that algorithm here. Relevant info: https://learn.microsoft.com/en-us/windows/win32/debug/pe-format

@steveharter steveharter modified the milestones: 9.0.0, 10.0.0 Jul 24, 2024
@steveharter steveharter added the help wanted [up-for-grabs] Good issue for external contributors label Jul 24, 2024
@teo-tsirpanis
Copy link
Contributor

I took a look at this. The reason reading the file did not fail, is that having failed to recognize the compatibility DOS header, the PE reader treated the file as a COFF file. COFF files however do not have a magic number in their header, and the ELF file was large enough to pass as having a COFF file header.

I don't think there is a reliable way to detect all cases of invalid files early, and this seems to be by design; PEReader reads the PE file lazily and cannot perform deep analysis firstly for performance reasons, and secondly because the file might contain data that PEReader does not know about.

@FiniteReality something you could do is adding an if (Enum.IsDefined(reader.PEHeaders.CoffHeader.Machine)) { throw new BadImageFormatException(); }. It's still not perfect but would catch your example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Reflection.Metadata enhancement Product code improvement that does NOT require public API changes/additions help wanted [up-for-grabs] Good issue for external contributors
Projects
No open projects
Development

No branches or pull requests

7 participants