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.

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

SerializationGuard is used when EnableUnsafeBinaryFormatterSerialization feature switch is disabled #44369

Open
marek-safar opened this issue Nov 6, 2020 · 4 comments
Labels
area-System.Runtime size-reduction Issues impacting final app size primary for size sensitive workloads
Milestone

Comments

@marek-safar
Copy link
Contributor

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 Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Nov 6, 2020
@Dotnet-GitSync-Bot
Copy link
Collaborator

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.

@marek-safar marek-safar added linkable-framework Issues associated with delivering a linker friendly framework area-Serialization labels Nov 6, 2020
@marek-safar marek-safar changed the title SerializationGuard is used when EnableUnsafeBinaryFormatterSerialization feature switch is disables SerializationGuard is used when EnableUnsafeBinaryFormatterSerialization feature switch is disabled Nov 6, 2020
@danmoseley
Copy link
Member

cc @GrabYourPitchforks

@GrabYourPitchforks
Copy link
Member

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

DeserializationToken deserializationToken = (_capturedLimiter != null) ? SerializationInfo.StartDeserialization() : default;
using (deserializationToken)
{
return SqlConvert.ChangeType2(argumentValues[0], mytype, type, FormatProvider);
}

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
Copy link
Contributor Author

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?

@marek-safar marek-safar added size-reduction Issues impacting final app size primary for size sensitive workloads and removed linkable-framework Issues associated with delivering a linker friendly framework labels Dec 9, 2020
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jul 12, 2021
@tannergooding tannergooding added this to the Future milestone Jul 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime size-reduction Issues impacting final app size primary for size sensitive workloads
Projects
None yet
Development

No branches or pull requests

6 participants