8000 Deprecate ObjectDataProvider · Issue #1199 · dotnet/wpf · GitHub
[go: up one dir, main page]
Skip to content

Deprecate ObjectDataProvider #1199

New issue

Have a question about this project 8000 ? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

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

Jump to bottom
Open
vatsan-madhavan opened this issue Jul 9, 2019 · 7 comments
Open

Deprecate ObjectDataProvider #1199

vatsan-madhavan opened this issue Jul 9, 2019 · 7 comments
Labels
Design Discussion Ongoing discussion about design without consensus Enhancement Requested Product code improvement that does NOT require public API changes/additions tenet-security For security related issues/PRs
Milestone
7.0.0

Comments

@vatsan-madhavan
Copy link
Member

We should consider marking ObjectDataProvider as [Obsolete] in .NET Core 3.

In addition to this, we should also consider a global policy switch (an AppContext switch, for e.g.) that will disable <ObjectDataProvider .../> in XAML.

Or perhaps this should be reversed - off by default and provide an opt-in switch? I worry that this will have a huge app-compat impact.

/cc @GrabYourPitchforks, @rladuca, @SamBent
/cc @grubioe
@grubioe grubioe added this to the 3.0 milestone Jul 11, 2019
@grubioe grubioe self-assigned this Jul 11, 2019
@grubioe grubioe added Design Discussion Ongoing discussion about design without consensus Enhancement Requested Product code improvement that does NOT require public API changes/additions tenet-security For security related issues/PRs labels Jul 11, 2019
@grubioe
Copy link
Contributor
grubioe commented Jul 11, 2019

WPF community - will removing ObjectDataProvider cause gaps with your application? We'd like your input to help formalize our approach. Thanks!

@weltkante
Copy link

We aren't using it in our codebase, but do you mind sharing the motivation for getting rid of it? I see you marked it as a security related issue, why is that?

Also I don't think the WPF community is represented well enough here to base the decision for removal in 3.0 off an issue discussion. You can of course try it (assuming you don't already get major objections here) and see if anyone else will run into problems, but you might have to add it back, especially since you put Desktop Framework compatibility on your 3.0 headlines.

@vatsan-madhavan
Copy link
Member Author

https://www.blackhat.com/docs/us-17/thursday/us-17-Munoz-Friday-The-13th-Json-Attacks.pdf

ObjectDataProvider provides a a clever approach to execute arbitrary methods.

@vatsan-madhavan
Copy link
Member Author

There are several approaches we can consider.

  1. Remove the type (heavy handed, will definitely break compat)
  2. Functionality no-op by default (i.e., the type will be available, can be compiled against etc., but the functionality will be a no-op), can be opted-in by applications that really want to use this type and related functionality
  3. Available by default (everything works as before), provide an opt-out switch (applications can opt-out of this in a way that makes all ObjectDataProvider related functionality a no-op)
  • Both (2) and (3) will retain compile/build time compat.
  • (2) will break runtime compat in the default configuration
  • (3) will preserve runtime compat also in the default configuration.

My personal preference is (2): no-op by default with an opt-in.

@weltkante
Copy link

Thanks for the link, and I agree with (2) being a reasonable choice.

However I'm not sure I can follow that argument about security yet, as far as I'm concerned (might be missing something) ObjectDataProvider is just another way to do data binding, I don't see anything that makes it special by e.g. consuming external data.

If this is about XAML loading, if you can load attacker controlled XAML you can already construct arbitrary objects anyways and put it e.g. into the Source of a Binding extension. So getting rid of just ObjectDataProvider doesn't make anything safer.

As far as I'm concerned if you (as an attacker) can create an ObjectDataProvider you probably also can create other objects. The primary use case for XAML in WPF is not serialization, if people use it for that there's probably plenty of security issues, I don't think dumbing everything in WPF down to make XAML a "safe" serialization format is a good idea. (Again, I might be missing something, so ignore that if thats the case.)

@grubioe grubioe added the rank20 Rank: Priority/rank on a scale of (1..100) label Jul 16, 2019
@vatsan-madhavan
Copy link
Member Author

We had some more discussion internally with @GrabYourPitchforks , and have decided to hold this for .NET Core 3.0 timeframe.

There is some hardening of deserialization in place since .NET Framework (#1132), and we will be removing BinaryFormatter uses in .NET 5(#1131).

We will revisit whether directly addressing ObjectDataProvider makes sense, or whether #1131 etc. is sufficient, in .NET 5.

@vatsan-madhavan vatsan-madhavan removed the rank20 Rank: Priority/rank on a scale of (1..100) label Jul 24, 2019
@vatsan-madhavan vatsan-madhavan modified the milestones: 3.0, 5.0 Jul 24, 2019
@grubioe grubioe removed their assignment Jul 24, 2019
@AceCoderLaura
Copy link

An application markup as powerful as XAML is inevitably going to be able to do things that could compromise security. If you're loading arbitrary XAML then you have to accept that security is going to be a trade off no matter what you do.

@ryalanms ryalanms modified the milestones: 5.0.0, 7.0.0 Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Design Discussion Ongoing discussion about design without consensus Enhancement Requested Product code improvement that does NOT require public API changes/additions tenet-security For security related issues/PRs
Projects
None yet
Milestone
7.0.0
Development

No branches or pull requests
5 participants
@weltkante @vatsan-madhavan @AceCoderLaura @ryalanms @grubioe
0