search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.27k stars 4.73k forks source link

SerializationGuard is used when EnableUnsafeBinaryFormatterSerialization feature switch is disabled #44369

Open marek-safar opened 4 years ago

marek-safar commented 4 years ago

SerializationGuard logic is still used even if EnableUnsafeBinaryFormatterSerialization feature is disabled via feature switch. This brings unnecessary dependencies for size sensitive workloads.

Ideally, code like

would be completely removed as well

Dotnet-GitSync-Bot commented 4 years ago

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.

danmoseley commented 4 years ago

cc @GrabYourPitchforks

GrabYourPitchforks commented 4 years ago

Serialization Guard isn't a BinaryFormatter-only thing. It's also currently used in DataSet / DataTable:

https://github.com/dotnet/runtime/blob/96073fd52739ecd8a97be800f23e4ebe5252fddd/src/libraries/System.Data.Common/src/System/Data/Filter/FunctionNode.cs#L515-L519

Interestingly, our implementation of Serialization Guard was the only thing that prevented the original CVE-2020-1147 PoC payload from executing correctly on .NET Core.

We've had offline discussions about enlightening other serializers like DataContractSerializer and System.Text.Json, and even third-party deserializers like JSON.NET. But TBH these discussions haven't really panned out.

The runtime crew generally likes the idea of nixing Serialization Guard entirely as a feature. I think eventually we can get there, but if we go this route it should be paired with a renewed effort to excise dangerous serialization code from the ecosystem.

marek-safar commented 4 years ago

It's also currently used in DataSet / DataTable

That API is probably something that will never be used in modern workloads. I'm not sure we actually have to have it at all for example on in the browser.

Secondly, someone would have to have Switch.System.Data.AllowArbitraryDataSetTypeInstantiation=true option set for the exploit to work, right? We can pair that with EnableUnsafeBinaryFormatterSerialization or some other feature if that's your concern.

if we go this route it should be paired with a renewed effort to excise dangerous serialization code from the ecosystem.

Are you pointing to source generators here?