Skip to content
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

Disallow unrestricted polymorphic deserialization in DataSet #39304

Merged
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -165,6 +165,7 @@
<data name="Data_ArgumentOutOfRange" xml:space="preserve"><value>'{0}' argument is out of range.</value></data>
<data name="Data_ArgumentNull" xml:space="preserve"><value>'{0}' argument cannot be null.</value></data>
<data name="Data_ArgumentContainsNull" xml:space="preserve"><value>'{0}' argument contains null value.</value></data>
<data name="Data_TypeNotAllowed" xml:space="preserve"><value>Type '{0}' is not allowed here. See https://go.microsoft.com/fwlink/?linkid=2132227 for more details.</value></data>
<data name="DataColumns_OutOfRange" xml:space="preserve"><value>Cannot find column {0}.</value></data>
<data name="DataColumns_Add1" xml:space="preserve"><value>Column '{0}' already belongs to this DataTable.</value></data>
<data name="DataColumns_Add2" xml:space="preserve"><value>Column '{0}' already belongs to another DataTable.</value></data>
Expand Down
23 changes: 13 additions & 10 deletions src/libraries/System.Data.Common/src/System.Data.Common.csproj
Expand Up @@ -124,6 +124,10 @@
<Compile Include="System\Data\KeyRestrictionBehavior.cs" />
<Compile Include="System\Data\LinqDataView.cs" />
<Compile Include="System\Data\LoadOption.cs" />
<Compile Include="System\Data\LocalAppContextSwitches.cs" />
<Compile Include="$(CommonPath)System\LocalAppContextSwitches.Common.cs">
<Link>Common\System\LocalAppContextSwitches.Common.cs</Link>
</Compile>
<Compile Include="System\Data\MappingType.cs" />
<Compile Include="System\Data\MergeFailedEvent.cs" />
<Compile Include="System\Data\MergeFailedEventHandler.cs" />
Expand Down Expand Up @@ -157,6 +161,7 @@
<Compile Include="System\Data\StrongTypingException.cs" />
<Compile Include="System\Data\TypedTableBase.cs" />
<Compile Include="System\Data\TypedTableBaseExtensions.cs" />
<Compile Include="System\Data\TypeLimiter.cs" />
<Compile Include="System\Data\UniqueConstraint.cs" />
<Compile Include="System\Data\UpdateRowSource.cs" />
<Compile Include="System\Data\Common\UInt64Storage.cs" />
Expand Down Expand Up @@ -296,25 +301,23 @@
<Compile Include="System\Data\ProviderBase\SchemaMapping.cs" />
</ItemGroup>
<ItemGroup>
<Reference Include="System.Collections" />
<ProjectReference Include="$(CoreLibProject)" />
<ProjectReference Include="..\..\System.Collections\src\System.Collections.csproj" />
<ProjectReference Include="..\..\System.Collections.NonGeneric\src\System.Collections.NonGeneric.csproj" />
<ProjectReference Include="..\..\System.ComponentModel.TypeConverter\src\System.ComponentModel.TypeConverter.csproj" />
<ProjectReference Include="..\..\System.Runtime\src\System.Runtime.csproj" />
<ProjectReference Include="..\..\System.Runtime.Extensions\src\System.Runtime.Extensions.csproj" />
<ProjectReference Include="..\..\System.Private.Uri\src\System.Private.Uri.csproj" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these stay as Reference?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because they reach into System.Private.CoreLib. We can only add <Reference> links to reference assemblies that are completely self-contained.

<Reference Include="System.Collections.Concurrent" />
<Reference Include="System.Collections.NonGeneric" />
<Reference Include="System.ComponentModel" />
<Reference Include="System.ComponentModel.Primitives" />
<Reference Include="System.ComponentModel.TypeConverter" />
<Reference Include="System.Diagnostics.Tracing" />
<Reference Include="System.Drawing.Primitives" />
<Reference Include="System.Linq" />
<Reference Include="System.Linq.Expressions" />
<Reference Include="System.Memory" />
<Reference Include="System.ObjectModel" />
<Reference Include="System.Runtime" />
<Reference Include="System.Runtime.Extensions" />
<Reference Include="System.Runtime.Numerics" />
<Reference Include="System.Runtime.Serialization.Formatters" />
<Reference Include="System.Text.Encoding.Extensions" />
<Reference Include="System.Text.RegularExpressions" />
<Reference Include="System.Threading" />
<Reference Include="System.Threading.Thread" />
<Reference Include="System.Transactions.Local" />
<Reference Include="System.Xml.ReaderWriter" />
<Reference Include="System.Xml.XmlSerializer" />
Expand Down
Expand Up @@ -403,6 +403,9 @@ public override object ConvertXmlToObject(XmlReader xmlReader, XmlRootAttribute

if (type == typeof(object))
throw ExceptionBuilder.CanNotDeserializeObjectType();

TypeLimiter.EnsureTypeIsAllowed(type);

if (!isBaseCLRType)
{
retValue = System.Activator.CreateInstance(type, true)!;
Expand Down
Expand Up @@ -143,6 +143,7 @@ public DataColumn(string? columnName, Type dataType, string? expr, MappingType t

private void UpdateColumnType(Type type, StorageType typeCode)
{
TypeLimiter.EnsureTypeIsAllowed(type);
_dataType = type;
_storageType = typeCode;
if (StorageType.DateTime != typeCode)
Expand Down
Expand Up @@ -350,6 +350,7 @@ private static void ThrowDataException(string error, Exception? innerException)
public static Exception ArgumentOutOfRange(string paramName) => _ArgumentOutOfRange(paramName, SR.Format(SR.Data_ArgumentOutOfRange, paramName));
public static Exception BadObjectPropertyAccess(string error) => _InvalidOperation(SR.Format(SR.DataConstraint_BadObjectPropertyAccess, error));
public static Exception ArgumentContainsNull(string paramName) => _Argument(paramName, SR.Format(SR.Data_ArgumentContainsNull, paramName));
public static Exception TypeNotAllowed(Type type) => _InvalidOperation(SR.Format(SR.Data_TypeNotAllowed, type.AssemblyQualifiedName));


//
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Data.Common/src/System/Data/DataSet.cs
Expand Up @@ -1960,9 +1960,11 @@ private void WriteXmlSchema(XmlWriter? writer, SchemaFormat schemaFormat, Conver

internal XmlReadMode ReadXml(XmlReader reader, bool denyResolving)
{
IDisposable? restrictedScope = null;
long logScopeId = DataCommonEventSource.Log.EnterScope("<ds.DataSet.ReadXml|INFO> {0}, denyResolving={1}", ObjectID, denyResolving);
try
{
restrictedScope = TypeLimiter.EnterRestrictedScope(this);
DataTable.DSRowDiffIdUsageSection rowDiffIdUsage = default;
try
{
Expand Down Expand Up @@ -2230,6 +2232,7 @@ internal XmlReadMode ReadXml(XmlReader reader, bool denyResolving)
}
finally
{
restrictedScope?.Dispose();
DataCommonEventSource.Log.ExitScope(logScopeId);
}
}
Expand Down Expand Up @@ -2466,9 +2469,11 @@ private void ReadXmlDiffgram(XmlReader reader)

internal XmlReadMode ReadXml(XmlReader? reader, XmlReadMode mode, bool denyResolving)
{
IDisposable? restictedScope = null;
long logScopeId = DataCommonEventSource.Log.EnterScope("<ds.DataSet.ReadXml|INFO> {0}, mode={1}, denyResolving={2}", ObjectID, mode, denyResolving);
try
{
restictedScope = TypeLimiter.EnterRestrictedScope(this);
XmlReadMode ret = mode;

if (reader == null)
Expand Down Expand Up @@ -2710,6 +2715,7 @@ internal XmlReadMode ReadXml(XmlReader? reader, XmlReadMode mode, bool denyResol
}
finally
{
restictedScope?.Dispose();
DataCommonEventSource.Log.ExitScope(logScopeId);
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/libraries/System.Data.Common/src/System/Data/DataTable.cs
Expand Up @@ -5657,9 +5657,11 @@ private bool IsEmptyXml(XmlReader reader)

internal XmlReadMode ReadXml(XmlReader? reader, bool denyResolving)
{
IDisposable? restrictedScope = null;
long logScopeId = DataCommonEventSource.Log.EnterScope("<ds.DataTable.ReadXml|INFO> {0}, denyResolving={1}", ObjectID, denyResolving);
try
{
restrictedScope = TypeLimiter.EnterRestrictedScope(this);
RowDiffIdUsageSection rowDiffIdUsage = default;
try
{
Expand Down Expand Up @@ -5894,15 +5896,18 @@ internal XmlReadMode ReadXml(XmlReader? reader, bool denyResolving)
}
finally
{
restrictedScope?.Dispose();
DataCommonEventSource.Log.ExitScope(logScopeId);
}
}

internal XmlReadMode ReadXml(XmlReader? reader, XmlReadMode mode, bool denyResolving)
{
IDisposable? restrictedScope = null;
RowDiffIdUsageSection rowDiffIdUsage = default;
try
{
restrictedScope = TypeLimiter.EnterRestrictedScope(this);
bool fSchemaFound = false;
bool fDataFound = false;
bool fIsXdr = false;
Expand Down Expand Up @@ -6188,6 +6193,7 @@ internal XmlReadMode ReadXml(XmlReader? reader, XmlReadMode mode, bool denyResol
}
finally
{
restrictedScope?.Dispose();
// prepare and cleanup rowDiffId hashtable
rowDiffIdUsage.Cleanup();
}
Expand Down
Expand Up @@ -5,6 +5,7 @@
using System.Data.Common;
using System.Data.SqlTypes;
using System.Diagnostics;
using System.Runtime.Serialization;

namespace System.Data
{
Expand All @@ -15,6 +16,7 @@ internal sealed class FunctionNode : ExpressionNode
internal int _argumentCount;
internal const int initialCapacity = 1;
internal ExpressionNode[]? _arguments;
private readonly TypeLimiter? _capturedLimiter;

private static readonly Function[] s_funcs = new Function[] {
new Function("Abs", FunctionId.Abs, typeof(object), true, false, 1, typeof(object), null, null),
Expand All @@ -39,6 +41,12 @@ internal sealed class FunctionNode : ExpressionNode

internal FunctionNode(DataTable? table, string name) : base(table)
{
// Because FunctionNode instances are created eagerly but evaluated lazily,
// we need to capture the deserialization scope here. The scope could be
// null if no deserialization is in progress.

_capturedLimiter = TypeLimiter.Capture();

_name = name;
for (int i = 0; i < s_funcs.Length; i++)
{
Expand Down Expand Up @@ -288,6 +296,11 @@ private Type GetDataType(ExpressionNode node)
throw ExprException.InvalidType(typeName);
}

// ReadXml might not be on the current call stack. So we'll use the TypeLimiter
// that was captured when this FunctionNode instance was created.

TypeLimiter.EnsureTypeIsAllowed(dataType, _capturedLimiter);

return dataType;
}

Expand Down Expand Up @@ -493,10 +506,17 @@ private object EvalFunction(FunctionId id, object[] argumentValues, DataRow? row
{
return SqlConvert.ChangeType2((decimal)SqlConvert.ChangeType2(argumentValues[0], StorageType.Decimal, typeof(decimal), FormatProvider), mytype, type, FormatProvider);
}
return SqlConvert.ChangeType2(argumentValues[0], mytype, type, FormatProvider);
}

return SqlConvert.ChangeType2(argumentValues[0], mytype, type, FormatProvider);
// The Convert function can be called lazily, outside of a previous Serialization Guard scope.
// If there was a type limiter scope on the stack at the time this Convert function was created,
// we must manually re-enter the Serialization Guard scope.

DeserializationToken deserializationToken = (_capturedLimiter != null) ? SerializationInfo.StartDeserialization() : default;
using (deserializationToken)
{
return SqlConvert.ChangeType2(argumentValues[0], mytype, type, FormatProvider);
}
}

return argumentValues[0];
Expand Down
@@ -0,0 +1,17 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.CompilerServices;

namespace System
{
internal static partial class LocalAppContextSwitches
{
private static int s_allowArbitraryTypeInstantiation;
public static bool AllowArbitraryTypeInstantiation
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get => GetCachedSwitchValue("Switch.System.Data.AllowArbitraryDataSetTypeInstantiation", ref s_allowArbitraryTypeInstantiation);
}
}
}