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.

Sign up for GitHub

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

Jump to bottom

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

Open
FiniteReality opened this issue Feb 17, 2021 · 4 comments · May be fixed by #113048
Open

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

FiniteReality opened this issue Feb 17, 2021 · 4 comments · May be fixed by #113048
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 in-pr There is an active PR which will close this issue when it is merged
Milestone
10.0.0

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
@FiniteReality FiniteReality mentioned this issue Feb 26, 2021
Closed
@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.

@teo-tsirpanis
Copy link
Contributor

Or, if you are certain that you are dealing with regular PE files, you can add an if (reader.IsCoffOnly) { throw new BadImageFormatException(); }. This is much more reliable than my previous suggestion, because PE files have magic numbers.

This was referenced Feb 23, 2025
Open
Open
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Mar 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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 in-pr There is an active PR which will close this issue when it is merged
Projects
No open projects
Triage POD for Reflection, META, etc.
Status: Future
Milestone
10.0.0
Development

Successfully merging a pull request may close this issue.

Perform additional checks in PEHeaders. teo-tsirpanis/dotnet-runtime
7 participants
@krwq @danmoseley @steveharter @ericstj @teo-tsirpanis @FiniteReality @buyaa-n