Fixing Microsoft Security Advisory CVE-2020-0605 : .NET Core Remote Code Execution Vulnerability (3.1 Fixed PR) #2428
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ternal overloads of XamlReader.Load to use RestrictiveXamlXmlReader Bugs: - Bug [1006083](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1006083): MSRC 54120: XAMLReader.Load used by `GetFixedDocumentSequence` method which could lead to code execution [.Net Core 3.1] - Bug [1006086](https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1006086): MSRC 54179: Code Execution using Malicious Annotation Files for Sticky Notes in WPF apps [.Net Core 3.1] ###Description Loose xaml can contain executable payload e.g. `ObjectDataProvider`. This Xaml can be included as part of `XpsDocument`s or base-64 encoded and then included in `StickyNote`s' annotation xml files. In WPF, we were allowing `XpsDocument`s and `StickyNote`s' annotation xml files to be loaded freely via `XamlReader.Load`. This exposes an attack vector - when a user downloads an XPS file from the internet for *viewing*, they could end up executing untrusted code. The fix is to identify known dangerous types and limit them from being deserialized during XAML loading. In order to accomplish this, we add new _non-public_ overloads to the `XamlReader.Load` method to enable the use of `RestrictiveXamlXmlReader`. `RestrictiveXamlXmlReader` restricts known dangerous types from being loaded while deserializing xaml. We then call `XamlReader.Load` via `XamlReaderProxy`, which is an adapter for `XamlReader` type and uses reflection to access `XamlReader.Load`. Reflection is used to avoid adding additional public surface area to `XamlReader` in servicing. Small changes are made to `TextRange` as well since the call-site for the `StickyNote`s case was through a call to `TextRange` which in turn calls into `XamlReader.Load`. ### Customer Impact Customers would be protected from opening potentially-compromised XPS documents and stickynotes annotation xml files. ### Regression No. This security issue was reported by an external party. ### Risk - Low - This change only affects loading XPS documents and loading stickynotes annotation data. - The change has been tested well internally. - We ran regression tests to ensure nothing else is inadvertently broken. - Validated against POC to ensure that the fix works as intended. In .NET Framework, we are introducing a quirk to give developers/cusotmers the option of going back to the old (i.e., unsecure) behavior where deserializing dangerous types like `ObjectDataProvider` will be allowed. In .NET Core, no quirks are being provided - we do not believe that this is a scenario that should be supported for compatibility in a relatively new platform.
ghost
requested a review
from 
ghost
added
the
vatsan-madhavan
approved these changes
Jan 15, 2020
|
note: do not squash when merging |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
@rladuca - empty change - so no azp run. You can just merge it directly IMO. Remember to create a merge-commit and not a squash-commit. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses issues #2424, #2425