Skip to content

Commit

Permalink
Implement Serialization Guard - CoreFX portion
Browse files Browse the repository at this point in the history
Consume Serialization Guard API in BinaryFormatter and Process.

Also includes Serialization Guard's tests.
  • Loading branch information
morganbr authored and bartonjs committed Apr 8, 2019
1 parent c653c89 commit 6ca1675
Show file tree
Hide file tree
Showing 10 changed files with 251 additions and 50 deletions.
1 change: 1 addition & 0 deletions src/System.Collections.Concurrent/src/Configurations.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
netcoreappaot-Windows_NT;
netcoreappaot-WebAssembly;
uap-Windows_NT;
uapaot-Windows_NT;
</BuildConfigurations>
</PropertyGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<AssemblyName>System.Collections.Concurrent</AssemblyName>
<RootNamespace>System.Collections.Concurrent</RootNamespace>
<IsPartialFacadeAssembly>true</IsPartialFacadeAssembly>
<Configurations>netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netcoreappaot-Windows_NT-Debug;netcoreappaot-Windows_NT-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release</Configurations>
<Configurations>netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netcoreappaot-WebAssembly-Debug;netcoreappaot-WebAssembly-Release;netcoreappaot-Windows_NT-Debug;netcoreappaot-Windows_NT-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release;uapaot-Windows_NT-Debug;uapaot-Windows_NT-Release</Configurations>
</PropertyGroup>
<ItemGroup>
<Compile Include="System\Collections\Concurrent\BlockingCollection.cs" />
Expand Down
28 changes: 28 additions & 0 deletions src/System.Diagnostics.Process/src/System/Diagnostics/Process.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
using System.ComponentModel;
using System.Globalization;
using System.IO;
using System.Reflection;
using System.Runtime.Serialization;
using System.Text;
using System.Threading;

Expand Down Expand Up @@ -81,6 +83,10 @@ public partial class Process : Component
internal bool _pendingOutputRead;
internal bool _pendingErrorRead;

private static int s_cachedSerializationSwitch = 0;
delegate void ThrowIfDeserializationInProgressWithSwitchDel(string switchName, ref int cachedValue);
private static ThrowIfDeserializationInProgressWithSwitchDel s_throwIfDeserializationInProgressWithSwitch = CreateThrowIfDeserializationInProgressWithSwitchDelegate();

/// <devdoc>
/// <para>
/// Initializes a new instance of the <see cref='System.Diagnostics.Process'/> class.
Expand Down Expand Up @@ -1173,6 +1179,23 @@ private void SetProcessId(int processId)
/// <summary>Additional optional configuration hook after a process ID is set.</summary>
partial void ConfigureAfterProcessIdSet();

/// <summary>
/// Builds a wrapper delegate for SerializationInfo.ThrowIfDeserializationInProgress(string, ref int),
/// since it is not exposed via contracts.
/// </summary>
private static ThrowIfDeserializationInProgressWithSwitchDel CreateThrowIfDeserializationInProgressWithSwitchDelegate()
{
ThrowIfDeserializationInProgressWithSwitchDel throwIfDeserializationInProgressDelegate = null;
MethodInfo throwMethod = typeof(SerializationInfo).GetMethod("ThrowIfDeserializationInProgress",
BindingFlags.Static | BindingFlags.NonPublic | BindingFlags.Public, null, new Type[] { typeof(string), typeof(int).MakeByRefType() }, new ParameterModifier[0]);
if (throwMethod != null)
{
throwIfDeserializationInProgressDelegate = (ThrowIfDeserializationInProgressWithSwitchDel)throwMethod.CreateDelegate(typeof(ThrowIfDeserializationInProgressWithSwitchDel));
}

return throwIfDeserializationInProgressDelegate;
}

/// <devdoc>
/// <para>
/// Starts a process specified by the <see cref='System.Diagnostics.Process.StartInfo'/> property of this <see cref='System.Diagnostics.Process'/>
Expand Down Expand Up @@ -1214,6 +1237,11 @@ public bool Start()
throw new ObjectDisposedException(GetType().Name);
}

if (s_throwIfDeserializationInProgressWithSwitch != null)
{
s_throwIfDeserializationInProgressWithSwitch("AllowProcessCreation", ref s_cachedSerializationSwitch);
}

return StartCore(startInfo);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
<Project DefaultTargets="Build">
<PropertyGroup>
<BuildConfigurations>
netcoreapp-Windows_NT;
netcoreapp-Unix;
uap-Windows_NT;
netcoreapp;
uapaot-Windows_NT;
netcoreappaot-Windows_NT;
netcoreappaot-WebAssembly;
$(PackageConfigurations);
</BuildConfigurations>
</PropertyGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
<Project Sdk="Microsoft.NET.Sdk">
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<ProjectGuid>{CF80D24A-787A-43DB-B9E7-10BCA02D10EA}</ProjectGuid>
<AssemblyName>System.Runtime.Serialization.Formatters</AssemblyName>
<RootNamespace>System.Runtime.Serialization.Formatters</RootNamespace>
<NoWarn>$(NoWarn);CS1573</NoWarn>
<Configurations>netcoreapp-Debug;netcoreapp-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release</Configurations>
<Configurations>netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;netcoreappaot-WebAssembly-Debug;netcoreappaot-WebAssembly-Release;netcoreappaot-Windows_NT-Debug;netcoreappaot-Windows_NT-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release;uapaot-Windows_NT-Debug;uapaot-Windows_NT-Release</Configurations>
</PropertyGroup>
<ItemGroup>
<Compile Include="System.Runtime.Serialization.Formatters.TypeForwards.cs" />
Expand Down Expand Up @@ -60,19 +60,14 @@
</Compile>
</ItemGroup>
<ItemGroup>
<Reference Include="System.Collections" />
<Reference Include="System.Collections.Concurrent" />
<Reference Include="System.Collections.NonGeneric" />
<Reference Include="System.Diagnostics.Debug" />
<Reference Include="System.Memory" />
<Reference Include="System.Resources.ResourceManager" />
<Reference Include="System.Runtime" />
<Reference Include="System.Runtime.CompilerServices.Unsafe" />
<Reference Include="System.Runtime.Extensions" />
<Reference Include="System.Text.Encoding.Extensions" />
<Reference Include="System.Threading" />
<ReferenceFromRuntime Include="System.Private.CoreLib" />
<ProjectReference Include="..\..\System.Runtime.CompilerServices.Unsafe\src\System.Runtime.CompilerServices.Unsafe.ilproj" />
<ProjectReference Include="..\..\System.Runtime\src\System.Runtime.csproj" />
<ProjectReference Include="..\..\System.Collections\src\System.Collections.csproj" />
<ProjectReference Include="..\..\System.Collections.Concurrent\src\System.Collections.Concurrent.csproj" />
<ProjectReference Include="..\..\System.Collections.NonGeneric\src\System.Collections.NonGeneric.csproj" />
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="Resources\System.Runtime.Serialization.Formatters.rd.xml" />
</ItemGroup>
</Project>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -87,45 +87,47 @@ internal object Deserialize(BinaryParser serParser, bool fCheck)

_isSimpleAssembly = (_formatterEnums._assemblyFormat == FormatterAssemblyStyle.Simple);


if (_fullDeserialization)
using (DeserializationToken token = SerializationInfo.StartDeserialization())
{
// Reinitialize
_objectManager = new ObjectManager(_surrogates, _context, false, false);
_serObjectInfoInit = new SerObjectInfoInit();
}
if (_fullDeserialization)
{
// Reinitialize
_objectManager = new ObjectManager(_surrogates, _context, false, false);
_serObjectInfoInit = new SerObjectInfoInit();
}

// Will call back to ParseObject, ParseHeader for each object found
serParser.Run();
// Will call back to ParseObject, ParseHeader for each object found
serParser.Run();

if (_fullDeserialization)
{
_objectManager.DoFixups();
}
if (_fullDeserialization)
{
_objectManager.DoFixups();
}

if (TopObject == null)
{
throw new SerializationException(SR.Serialization_TopObject);
}
if (TopObject == null)
{
throw new SerializationException(SR.Serialization_TopObject);
}

//if TopObject has a surrogate then the actual object may be changed during special fixup
//So refresh it using topID.
if (HasSurrogate(TopObject.GetType()) && _topId != 0)//Not yet resolved
{
TopObject = _objectManager.GetObject(_topId);
}
//if TopObject has a surrogate then the actual object may be changed during special fixup
//So refresh it using topID.
if (HasSurrogate(TopObject.GetType()) && _topId != 0)//Not yet resolved
{
TopObject = _objectManager.GetObject(_topId);
}

if (TopObject is IObjectReference)
{
TopObject = ((IObjectReference)TopObject).GetRealObject(_context);
}
if (TopObject is IObjectReference)
{
TopObject = ((IObjectReference)TopObject).GetRealObject(_context);
}

if (_fullDeserialization)
{
_objectManager.RaiseDeserializationEvent(); // This will raise both IDeserialization and [OnDeserialized] events
}
if (_fullDeserialization)
{
_objectManager.RaiseDeserializationEvent(); // This will raise both IDeserialization and [OnDeserialized] events
}

return TopObject;
return TopObject;
}
}
private bool HasSurrogate(Type t)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -773,8 +773,17 @@ internal void CompleteISerializableObject(object obj, SerializationInfo info, St
{
throw new SerializationException(SR.Format(SR.Serialization_ConstructorNotFound, t), e);
}
try
{
constInfo.Invoke(obj, new object[] { info, context });
}
// This will only throw TargetInvocationExceptions, but to provide a better exception for dangerous deserialization, unwrap that
// and re-wrap it in a DeserializationBlockedException (while preserving its stack)
catch (TargetInvocationException outerException) when (outerException.InnerException is DeserializationBlockedException)
{
throw new DeserializationBlockedException(outerException.InnerException);
}

constInfo.Invoke(obj, new object[] { info, context });
}

internal static ConstructorInfo GetDeserializationConstructor(Type t)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,154 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Diagnostics;
using System.IO;
using System.Reflection;
using System.Runtime.Serialization.Formatters.Binary;
using System.Threading;
using System.Threading.Tasks;
using Xunit;

namespace System.Runtime.Serialization.Formatters.Tests
{
public static class SerializationGuardTests
{
[Fact]
public static void BlockAssemblyLoads()
{
TryPayload(new AssemblyLoader());
}

[Fact]
public static void BlockProcessStarts()
{
TryPayload(new ProcessStarter());
}

[Fact]
public static void BlockFileWrites()
{
TryPayload(new FileWriter());
}

[Fact]
public static void BlockReflectionDodging()
{
// Ensure that the deserialization tracker cannot be called by reflection.
MethodInfo trackerMethod = typeof(Thread).GetMethod(
"GetThreadDeserializationTracker",
BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static);

Assert.NotNull(trackerMethod);

Assert.Equal(1, trackerMethod.GetParameters().Length);
object[] args = new object[1];
args[0] = Enum.ToObject(typeof(Thread).Assembly.GetType("System.Threading.StackCrawlMark"), 0);

try
{
object tracker = trackerMethod.Invoke(null, args);
throw new InvalidOperationException(tracker?.ToString() ?? "(null tracker returned)");
}
catch (TargetInvocationException ex)
{
Exception baseEx = ex.GetBaseException();
AssertExtensions.Throws<ArgumentException>("stackMark", () => throw baseEx);
}
}

[Fact]
public static void BlockAsyncDodging()
{
TryPayload(new AsyncDodger());
}

private static void TryPayload(object payload)
{
MemoryStream ms = new MemoryStream();
BinaryFormatter writer = new BinaryFormatter();
writer.Serialize(ms, payload);
ms.Position = 0;

BinaryFormatter reader = new BinaryFormatter();
Assert.Throws<DeserializationBlockedException>(() => reader.Deserialize(ms));
}
}

[Serializable]
internal class AssemblyLoader : ISerializable
{
public AssemblyLoader() { }

public AssemblyLoader(SerializationInfo info, StreamingContext context)
{
Assembly.Load(new byte[1000]);
}

public void GetObjectData(SerializationInfo info, StreamingContext context)
{
}
}

[Serializable]
internal class ProcessStarter : ISerializable
{
public ProcessStarter() { }

private ProcessStarter(SerializationInfo info, StreamingContext context)
{
Process.Start("calc.exe");
}

public void GetObjectData(SerializationInfo info, StreamingContext context)
{
}
}

[Serializable]
internal class FileWriter : ISerializable
{
public FileWriter() { }

private FileWriter(SerializationInfo info, StreamingContext context)
{
string tempPath = Path.GetTempFileName();
File.WriteAllText(tempPath, "This better not be written...");
throw new InvalidOperationException("Unreachable code (SerializationGuard should have kicked in)");
}

public void GetObjectData(SerializationInfo info, StreamingContext context)
{
}
}

[Serializable]
internal class AsyncDodger : ISerializable
{
public AsyncDodger() { }

private AsyncDodger(SerializationInfo info, StreamingContext context)
{
try
{
Task t = Task.Factory.StartNew(LoadAssemblyOnBackgroundThread, TaskCreationOptions.LongRunning);
t.Wait();
}
catch (AggregateException ex)
{
throw ex.InnerException;
}
}

private void LoadAssemblyOnBackgroundThread()
{
Assembly.Load(new byte[1000]);
}

public void GetObjectData(SerializationInfo info, StreamingContext context)
{
}
}
}
Loading

0 comments on commit 6ca1675

Please sign in to comment.