Skip to content

Commit

Permalink
Remove support for UseLegacyDangerousClipboardDeserializationMode (#…
Browse files Browse the repository at this point in the history
…1286)

* Remove support for `UseLegacyDangerousClipboardDeserializationMode` and permanently disallow deserialization of dangerous types.
  • Loading branch information
vatsan-madhavan committed Jul 23, 2019
1 parent 574e374 commit a9b5c09
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -337,25 +337,6 @@ public static bool AllowExternalProcessToBlockAccessToTemporaryFiles

#endregion

#region EnableLegacyDangerousClipboardDeserializationMode

internal const string EnableLegacyDangerousClipboardDeserializationModeSwitchName = "Switch.System.Windows.EnableLegacyDangerousClipboardDeserializationMode";
private static int _enableLegacyDangerousClipboardDeserializationMode;
public static bool EnableLegacyDangerousClipboardDeserializationMode
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get
{
/// <summary>
/// Malicious managed objects could be placed in the clipboard lying about its format,
/// to fix this OleConverter now restricts object deserialization in some cases.
/// When this switch is enabled behavior falls back to deserializing without restriction.
/// </summary>
return LocalAppContext.GetCachedSwitchValue(EnableLegacyDangerousClipboardDeserializationModeSwitchName, ref _enableLegacyDangerousClipboardDeserializationMode);
}
}

#endregion
}
#pragma warning restore 436
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ private static void InitializeNetFxSwitchDefaultsForNetCoreRuntime()
LocalAppContext.DefineSwitchDefault(CoreAppContextSwitches.ShouldNotRenderInNonInteractiveWindowStationSwitchName, false);
LocalAppContext.DefineSwitchDefault(CoreAppContextSwitches.DoNotUsePresentationDpiCapabilityTier3OrGreaterSwitchName, false);
LocalAppContext.DefineSwitchDefault(CoreAppContextSwitches.AllowExternalProcessToBlockAccessToTemporaryFilesSwitchName, false);
LocalAppContext.DefineSwitchDefault(CoreAppContextSwitches.EnableLegacyDangerousClipboardDeserializationModeSwitchName, false);
}
}
#pragma warning restore 436
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,17 +490,6 @@ public static void SetDataObject(object data, bool copy)
//
//------------------------------------------------------

/// <summary>
/// Determines whether the legacy dangerous clipboard deserialization mode should be used based on the AppContext switch and Device Guard policies.
/// </summary>
/// <returns>
/// If Device Guard is enabled this method returns false, otherwise it returns the AppContext switch value.
/// </returns>
internal static bool UseLegacyDangerousClipboardDeserializationMode()
{
return !IsDeviceGuardEnabled && CoreAppContextSwitches.EnableLegacyDangerousClipboardDeserializationMode;
}

/// <summary>
/// Places data on the system Clipboard and uses copy to specify whether the data
/// should remain on the Clipboard after the application exits.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2863,13 +2863,33 @@ private object GetDataFromHGLOBAL(string format, IntPtr hglobal)
{
data = ReadBitmapSourceFromHandle(hglobal);
}
// Restrict deserialization to only primitives
// and strings here to prevent potentially malicious objects from
// Limit deserialization to DataFormats that correspond to primitives, which are:
//
// DataFormats.CommaSeparatedValue
// DataFormats.FileDrop
// DataFormats.Html
// DataFormats.OemText
// DataFormats.PenData
// DataFormats.Rtf
// DataFormats.Serializable
// DataFormats.Text
// DataFormats.UnicodeText
// DataFormats.WaveAudio
// DataFormats.Xaml
// DataFormats.XamlPackage
// DataFormats.StringFormat *
//
// * Out of these, we will disallow deserialization of
// DataFormats.StringFormat to prevent potentially malicious objects from
// being deserialized as part of a "text" copy-paste or drag-drop.
// TypeRestrictingSerializationBinder will throw when it encounters
// anything other than strings and primitives - this ensures that we will
// continue successfully deserializing basic strings while rejecting other
// data types that advertise themselves as DataFormats.StringFormat.
//
// The rest of the following formats are pre-defined in the OS,
// they are not managed objects so we shouldn't try to deserialize them as such,
// allow primitives in a best effort for compat, but restrict other types.
else if (!Clipboard.UseLegacyDangerousClipboardDeserializationMode())
// they are not managed objects - an so we will not attempt to deserialize them.
else
{
bool restrictDeserialization =
(IsFormatEqual(format, DataFormats.StringFormat) ||
Expand All @@ -2888,11 +2908,7 @@ private object GetDataFromHGLOBAL(string format, IntPtr hglobal)

data = ReadObjectFromHandle(hglobal, restrictDeserialization);
}
else
{
data = ReadObjectFromHandle(hglobal, restrictDeserialization: false);
}
}
}

return data;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ protected override void DoPaste(IDataObject dataObject)

if ( !String.IsNullOrEmpty(xml) )
{
bool useRestrictiveXamlReader = !Clipboard.UseLegacyDangerousClipboardDeserializationMode();
UIElement element = XamlReader.Load(new System.Xml.XmlTextReader(new System.IO.StringReader(xml)), useRestrictiveXamlReader) as UIElement;
UIElement element = XamlReader.Load(new System.Xml.XmlTextReader(new System.IO.StringReader(xml)), useRestrictiveXamlReader: true) as UIElement;
if (element != null)
{
ElementList.Add(element);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -923,8 +923,7 @@ private static bool PasteXaml(TextEditor This, string pasteXaml)
try
{
// Parse the fragment into a separate subtree
bool useRestrictiveXamlReader = !Clipboard.UseLegacyDangerousClipboardDeserializationMode();
object xamlObject = XamlReader.Load(new XmlTextReader(new System.IO.StringReader(pasteXaml)), useRestrictiveXamlReader);
object xamlObject = XamlReader.Load(new XmlTextReader(new System.IO.StringReader(pasteXaml)), useRestrictiveXamlReader: true);
TextElement flowContent = xamlObject as TextElement;

success = flowContent == null ? false : PasteTextElement(This, flowContent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,8 +344,7 @@ internal static object LoadElement(Stream stream)
parserContext.BaseUri = entryPartUri;

// Call xaml parser
bool useRestrictiveXamlReader = !Clipboard.UseLegacyDangerousClipboardDeserializationMode();
xamlObject = XamlReader.Load(xamlEntryPart.GetSeekableStream(), parserContext, useRestrictiveXamlReader);
xamlObject = XamlReader.Load(xamlEntryPart.GetSeekableStream(), parserContext, useRestrictiveXamlReader: true);

// Remove the temporary uri from the PackageStore
PackageStore.RemovePackage(packageUri);
Expand Down

0 comments on commit a9b5c09

Please sign in to comment.