Skip to content

Commit 57f8db5

Browse files
authored
Merge pull request #2430 from dotnet/dev/arpit/release_1b
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`.
2 parents 457b965 + 740fbe5 commit 57f8db5

File tree

6 files changed

+192
-68
lines changed

6 files changed

+192
-68
lines changed

src/Microsoft.DotNet.Wpf/cycle-breakers/PresentationFramework/PresentationFramework.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11407,13 +11407,20 @@ public void CancelAsync() { }
1140711407
public static System.Xaml.XamlSchemaContext GetWpfSchemaContext() { throw null; }
1140811408
public static object Load(System.IO.Stream stream) { throw null; }
1140911409
public static object Load(System.IO.Stream stream, System.Windows.Markup.ParserContext parserContext) { throw null; }
11410+
public static object Load(System.IO.Stream stream, System.Windows.Markup.ParserContext parserContext, bool useRestrictiveXamlReader) { throw null; }
1141011411
public static object Load(System.Xaml.XamlReader reader) { throw null; }
1141111412
public static object Load(System.Xml.XmlReader reader) { throw null; }
11413+
public static object Load(System.Xml.XmlReader reader, bool useRestrictiveXamlReader) { throw null; }
1141211414
public object LoadAsync(System.IO.Stream stream) { throw null; }
11415+
public object LoadAsync(System.IO.Stream stream, bool useRestrictiveXamlReader) { throw null; }
1141311416
public object LoadAsync(System.IO.Stream stream, System.Windows.Markup.ParserContext parserContext) { throw null; }
11417+
public object LoadAsync(System.IO.Stream stream, System.Windows.Markup.ParserContext parserContext, bool useRestrictiveXamlReader) { throw null; }
1141411418
public object LoadAsync(System.Xml.XmlReader reader) { throw null; }
11419+
public object LoadAsync(System.Xml.XmlReader reader, bool useRestrictiveXamlReader) { throw null; }
1141511420
public static object Parse(string xamlText) { throw null; }
11421+
public static object Parse(string xamlText, bool useRestrictiveXamlReader) { throw null; }
1141611422
public static object Parse(string xamlText, System.Windows.Markup.ParserContext parserContext) { throw null; }
11423+
public static object Parse(string xamlText, System.Windows.Markup.ParserContext parserContext, bool useRestrictiveXamlReader) { throw null; }
1141711424
}
1141811425
public partial class XamlTypeMapper
1141911426
{

src/Microsoft.DotNet.Wpf/src/PresentationFramework/MS/Internal/Controls/StickyNote/StickyNoteContentControl.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ public override void Load(XmlNode node)
234234
RichTextBox richTextBox = (RichTextBox)InnerControl;
235235

236236
FlowDocument document = new FlowDocument();
237-
TextRange rtbRange = new TextRange(document.ContentStart, document.ContentEnd);
237+
TextRange rtbRange = new TextRange(document.ContentStart, document.ContentEnd, useRestrictiveXamlXmlReader: true);
238238
using (MemoryStream buffer = new MemoryStream(Convert.FromBase64String(node.InnerText)))
239239
{
240240
rtbRange.Load(buffer, DataFormats.Xaml);

src/Microsoft.DotNet.Wpf/src/PresentationFramework/System/Windows/Documents/TextRange.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,24 @@ public TextRange(TextPointer position1, TextPointer position2) :
6565
internal TextRange(ITextPointer position1, ITextPointer position2)
6666
: this(position1, position2, false /* ignoreTextUnitBoundaries */)
6767
{
68+
}
69+
70+
/// <summary>
71+
/// Creates a new TextRange instance.
72+
/// </summary>
73+
/// <param name="position1">
74+
/// </param>
75+
/// TextPointer specifying the static end of the new TextRange.
76+
/// <param name="position2">
77+
/// TextPointer specifying the dynamic end of the new TextRange.
78+
/// </param>
79+
/// <param name="useRestrictiveXamlXmlReader">
80+
/// Boolean flag. False by default, set to true to disable external xaml loading in specific scenarios like StickyNotes annotation loading
81+
/// </param>
82+
internal TextRange(TextPointer position1, TextPointer position2, bool useRestrictiveXamlXmlReader) :
83+
this((ITextPointer)position1, (ITextPointer)position2)
84+
{
85+
_useRestrictiveXamlXmlReader = useRestrictiveXamlXmlReader;
6886
}
6987

7088
// ignoreTextUnitBoundaries - true if normalization should ignore text
@@ -1366,7 +1384,7 @@ internal string Xml
13661384
try
13671385
{
13681386
// Parse the fragment into a separate subtree
1369-
object xamlObject = XamlReader.Load(new XmlTextReader(new System.IO.StringReader(value)));
1387+
object xamlObject = XamlReader.Load(new XmlTextReader(new System.IO.StringReader(value)), _useRestrictiveXamlXmlReader);
13701388
TextElement fragment = xamlObject as TextElement;
13711389

13721390
if (fragment != null)
@@ -1900,6 +1918,9 @@ private enum Flags
19001918
// Boolean flags, set with Flags enum.
19011919
private Flags _flags;
19021920

1921+
// Boolean flag, set to true via constructor when you want to use the RestrictiveXamlXmlReader
1922+
private bool _useRestrictiveXamlXmlReader;
1923+
19031924
#endregion Private Fields
19041925
}
19051926
}

0 commit comments

Comments
 (0)