ResourceEditor should not serialize MemoryStreams using BinaryFormatter

[ericstj]

Visual Studio Version:
16.3.0 Preview 2.0

Summary:
See the scenario here: https://github.com/dotnet/coreclr/issues/26472#issuecomment-528112760

Steps to Reproduce:

1. Add a sound file to resource collection using the resources editor.

2. Select Embedded in .resx for the Persistence option.

Expected Behavior:
Sound file is embedded as a "stream" primitive in the resources. When read at runtime caller will get a stream over top of the embedded resource's bytes.

Actual Behavior:
Resource editor is writing a binary formatted MemoryStream. MemoryStream is not serializable on all frameworks, so this leads to an exception when trying to deserialize the MemoryStream on .NETCore.

User Impact:
User cannot store a sounds embedded in ResX on .NETCore.

[ericstj]

/cc @rainersigwald

[davkean]

If not a memory stream, then what? What is a "stream" primitive?

[ericstj]

When you call ResourceWriter with AddResource(string,Stream) or AddResource(string,Stream as object) it gets stored as the backing byte-array of the stream, then deserialized at runtime as an unmanagedmemorystream over the raw bytes in the dll mapped in memory.

@rainersigwald does ResGen have any paths where it will new up a stream for MemoryStream for something embedded in the resx? I thought we did, but looking now I don't see anything for the embedded one:
https://github.com/microsoft/msbuild/blob/3885a205f40944174a3e14396547f13db1196b82/src/Tasks/ResourceHandling/MSBuildResXReader.cs#L164-L199

Even if we did, we'd need ResXResourceWriter to also write in that different format, since anything written as binaryFormatted memory stream will not deserialize on core. So I'm guessing to fix this we'd need it to be written as application/x-microsoft.net.object.bytearray.base64 of MemoryStream type, then ResGen would need to special case MemoryStream and activate it before calling AddResource.

[tmeschter]

@ericstj I'm trying to understand the impact of this bug. Is this a security issue, a functional issue, or something else?

[verelpode]

Currently in VS 16.8.3, if you add a binary file to a .resx, and the file is NOT .wav, such as if you add a test file named "Test.abc" where ".abc" is NOT .wav, then the resx Designer represents it using System.Byte[], which would be great, but unfortunately it includes references to the WinForms DLL System.Windows.Forms.dll. Currently it produces:

<assembly alias="System.Windows.Forms" name="System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089" />
<data name="DgmlGraph" type="System.Resources.ResXFileRef, System.Windows.Forms">
	<value>Resources\DgmlGraph.png;System.Byte[], mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
</data>

It would be great if sometime the Designer/GUI for .resx could be updated for NETFW 5 and modernized to be independent of WinForms. RESX is useful but currently I avoid using it because of the problematic reference to the obsolete WinForms. Obviously in all new projects I don't want to create any dependencies on WinForms.

AFAIK, .resx does not require WinForms, rather the issue is that the Designer/GUI for it makes references to WinForms.

Unfortunately System.Resources.ResXFileRef is in the DLL "System.Windows.Forms.dll", but NETFW 5 could provide RESX stuff independently of WinForms.

[tmeschter]

@verelpode That's a separate issue from what is being discussed here. Can you open a new issue (in this repo) to track that request?

[ericstj]

@ericstj I'm trying to understand the impact of this bug. Is this a security issue, a functional issue, or something else?

Functional issue. See the linked issue dotnet/runtime#13349 (comment). MemoryStream is not serializable on .NETCore so if the designer is storing resources as serialized streams they will not be consumable on .NETCore.

The fix here is to ensure the designer is writing using https://docs.microsoft.com/en-us/dotnet/api/system.resources.resourcewriter.addresource?view=net-5.0#System_Resources_ResourceWriter_AddResource_System_String_System_IO_Stream_

This is related to the effort in which folks desire to completely remove BinaryFormatted resources: dotnet/runtime#39292 Which is a part of larger effort to obsolete BinaryFormatter.

[tmeschter]

@ericstj The Resource Editor currently uses ResXResourceReader/ResXResourceWriter, since it operates on .resx files. I don't think we can use that ResourceWriter.AddResource overload.