BinaryFormatter is insecure and strongly cautioned against

[jonko0493]

Not a bug, so eschewing the template for now. The DragDropLib uses the BinaryFormatter class. The .NET folks recommend against this strongly and have even begun disabling it in more recent versions of .NET. It looks like we only use it in this one place, so it shouldn't be super hard to move off of, I'd guess.

[jonko0493]

I think the fix here is pretty simple. Pretty sure you can seamlessly replace the two spots it's used with:

DataContractSerializer typeSerializer = new DataContractSerializer(typeof(Type));
typeSerializer.WriteObject(stream, data.GetType());
DataContractSerializer objectSerializer = new DataContractSerializer(data.GetType());
objectSerializer.WriteObject(stream, GetAsSerializable(data));

and

DataContractSerializer typeSerializer = new DataContractSerializer(typeof(Type));
Type dataType = (Type)typeSerializer.ReadObject(dataStream);
DataContractSerializer objectSerializer = new DataContractSerializer(dataType);
object data2 = objectSerializer.ReadObject(dataStream);

for serialization/deserialization, respectively.

[jonko0493]

So the way this initially surfaced was that BinaryFormatter usage causes crashes by default in net8.0. After upgrading to it, we started seeing these crashes. While I tested this in Eto.Test, I foolishly forgot that that project is, of course, on net6.0, so we're not gonna see those crashes happen.

BinaryFormatter serialization and deserialization are disabled within this application. See https://aka.ms/binaryformatter for more information.

   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph)
   at System.Windows.DataObject.SaveObjectToHandle(IntPtr handle, Object data, Boolean doNotReallocate)
   at System.Windows.DataObject.GetDataIntoOleStructsByTypeMedimHGlobal(String format, Object data, STGMEDIUM& medium, Boolean doNotReallocate)
   at System.Windows.DataObject.GetDataIntoOleStructs(FORMATETC& formatetc, STGMEDIUM& medium, Boolean doNotReallocate)
   at System.Windows.DataObject.System.Runtime.InteropServices.ComTypes.IDataObject.GetData(FORMATETC& formatetc, STGMEDIUM& medium)
   at System.Windows.WpfDataObjectExtensions.SetDataEx(IDataObject dataObject, String format, Object data)
   at Eto.Wpf.Forms.DataObjectHandler`2.SetString(String value, String type)
   at Eto.Forms.DataObject.SetString(String value, String type)
   at Eto.Forms.DataObject.SetObject(Object value, String type)
   at SerialLoops.Controls.ScriptCommandSectionTreeGridView.OnMouseMove(Object sender, MouseEventArgs e) in C:\Users\User\source\repos\SerialLoops\src\SerialLoops\Controls\ScriptCommandSectionTree.cs:line 216
   at Eto.Forms.Control.Callback.OnMouseMove(Control widget, MouseEventArgs e)
   at Eto.Wpf.Forms.WpfFrameworkElement`3.HandleMouseMove(Object sender, MouseEventArgs e)
   at System.Windows.RoutedEventArgs.InvokeHandler(Delegate handler, Object target)
   at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised)
   at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args)
   at System.Windows.UIElement.RaiseTrustedEvent(RoutedEventArgs args)
   at System.Windows.Input.InputManager.ProcessStagingArea()
   at System.Windows.Interop.HwndMouseInputProvider.ReportInput(IntPtr hwnd, InputMode mode, Int32 timestamp, RawMouseActions actions, Int32 x, Int32 y, Int32 wheel)
   at System.Windows.Interop.HwndMouseInputProvider.FilterMessage(IntPtr hwnd, WindowMessage msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at System.Windows.Interop.HwndSource.InputFilterMessage(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Application.RunDispatcher(Object ignore)
   at System.Windows.Application.RunInternal(Window window)
   at Eto.Wpf.Forms.ApplicationHandler.Run()
   at Eto.Forms.Application.Run(Form mainForm)
   at SerialLoops.Wpf.Program.Main(String[] args) in C:\Users\User\source\repos\SerialLoops\src\SerialLoops.Wpf\Program.cs:line 56

Looking at this stack trace in more detail, though, we can see that this is actually coming from a COM interop call here into some WPF libraries directly. Following the stack trace down, we eventually come to this method which we can see does, in fact, use BinaryFormatter.

Anyway, suffice to say: I'd recommend reopening this because this is still going to cause crashes out of the box when upgrading to net8.0, but it's a bigger issue than I initially realized and will require some thought to fix.

cc/ @cwensley

[cwensley]

Anyway, suffice to say: I'd recommend reopening this because this is still going to cause crashes out of the box when upgrading to net8.0, but it's a bigger issue than I initially realized and will require some thought to fix.

Hey @jonko0493, thanks for digging into this! I would recommend opening an issue at http://github.com/dotnet/wpf instead, as there's nothing we can do to fix it here.

Cheers,
Curtis.

[jonko0493]

@cwensley yeah, I was starting to think this might be something to talk to the .NET folks about. I'll reach out to them :)

[jonko0493]

Looks like they have an open issue about it here: dotnet/wpf#1131

Seems that they were planning to remove it in the .NET 5 timeline but never got around to it.