Skip to content

Remove support for UseLegacyDangerousClipboardDeserializationMode #1286

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

Merged
merged 4 commits into from
Jul 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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