dotnet / winforms

Windows Forms is a .NET UI framework for building Windows desktop applications.
MIT License
4.42k stars 984 forks source link

Consider replacing the use of BinaryFormatter at all in WinForms. #10911

Closed AraHaan closed 8 months ago

AraHaan commented 8 months ago

.NET version

I will investigate a way to be able to parse the types from Resx without the security issues from BinaryFormatter inside of the winforms codebase in the next few weeks. Because .NET 9 will entirely remove them by default and that I FEEL LIKE being REQUIRED to reference an external nuget package because I set "Localizable" to true on a form should not justify a possible security hole in people's applications. That is not acceptable.

Did it work in .NET Framework?

Yes

Did it work in any of the earlier releases of .NET Core or .NET 5+?

Yes, but BinaryFormatter was deprecated. Now in .NET 9 it will throw an exception on all of it's APIs, and will have the real code in a nuget package. Although best option here is to look into other means of doing the same thing from within the Windows Forms repository. But first I will write code separate from it to test and then copy paste it all into the Windows Forms codebase to be used internally as an implementation detail.

Issue description

BinaryFormatter should be avoided entirely, I know there are other ways of the same thing, perhaps serialize the types as json instead and store said json strings inside of the resx and deserialize that instead at runtime? Would still need code in the designer for that however to migrate the format of those strings in the resx files to json form but it can be done like that as well. Also System.Text.Json is part of the main shared framework and so it should be no problems depending on it.

Steps to reproduce

AraHaan commented 8 months ago

https://github.com/search?q=repo%3Adotnet%2Fwinforms%20BinaryFormatter&type=code for where it is used in the code today.

elachlan commented 8 months ago

@lonitra I think has been working on this area.

AraHaan commented 8 months ago

I see, I think it is best to see what they have in mind then as well.

merriemcgaw commented 8 months ago

Thank you for filing this issue! I'm going to consider this a duplicate of the epic that @lonitra and @JeremyKuhne are working on #6267