Disallow unrestricted polymorphic deserialization in DataSet (#39304)

Fixes CVE-2020-1147 https://portal.msrc.microsoft.com/en-us/security-guidance/advisory/CVE-2020-1147 See also https://go.microsoft.com/fwlink/?linkid=2132227.
dotnet · Jul 15, 2020 · 37cf387 · 37cf387
1 parent 324c6c7
commit 37cf387
Show file tree

Hide file tree

Showing 12 changed files with 820 additions and 12 deletions.
diff --git a/src/libraries/System.Data.Common/src/Resources/Strings.resx b/src/libraries/System.Data.Common/src/Resources/Strings.resx
@@ -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>

diff --git a/src/libraries/System.Data.Common/src/System.Data.Common.csproj b/src/libraries/System.Data.Common/src/System.Data.Common.csproj
@@ -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" />
@@ -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" />
@@ -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" />
     <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" />

diff --git a/src/libraries/System.Data.Common/src/System/Data/Common/ObjectStorage.cs b/src/libraries/System.Data.Common/src/System/Data/Common/ObjectStorage.cs
@@ -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)!;

diff --git a/src/libraries/System.Data.Common/src/System/Data/DataColumn.cs b/src/libraries/System.Data.Common/src/System/Data/DataColumn.cs
@@ -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)

diff --git a/src/libraries/System.Data.Common/src/System/Data/DataException.cs b/src/libraries/System.Data.Common/src/System/Data/DataException.cs
@@ -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));
 
 
         //

diff --git a/src/libraries/System.Data.Common/src/System/Data/DataSet.cs b/src/libraries/System.Data.Common/src/System/Data/DataSet.cs
@@ -1964,9 +1964,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
                 {
@@ -2234,6 +2236,7 @@ internal XmlReadMode ReadXml(XmlReader reader, bool denyResolving)
             }
             finally
             {
+                restrictedScope?.Dispose();
                 DataCommonEventSource.Log.ExitScope(logScopeId);
             }
         }
@@ -2470,9 +2473,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)
@@ -2714,6 +2719,7 @@ internal XmlReadMode ReadXml(XmlReader? reader, XmlReadMode mode, bool denyResol
             }
             finally
             {
+                restictedScope?.Dispose();
                 DataCommonEventSource.Log.ExitScope(logScopeId);
             }
         }

diff --git a/src/libraries/System.Data.Common/src/System/Data/DataTable.cs b/src/libraries/System.Data.Common/src/System/Data/DataTable.cs
@@ -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
                 {
@@ -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;
@@ -6188,6 +6193,7 @@ internal XmlReadMode ReadXml(XmlReader? reader, XmlReadMode mode, bool denyResol
             }
             finally
             {
+                restrictedScope?.Dispose();
                 // prepare and cleanup rowDiffId hashtable
                 rowDiffIdUsage.Cleanup();
             }

diff --git a/src/libraries/System.Data.Common/src/System/Data/Filter/FunctionNode.cs b/src/libraries/System.Data.Common/src/System/Data/Filter/FunctionNode.cs
@@ -5,6 +5,7 @@
 using System.Data.Common;
 using System.Data.SqlTypes;
 using System.Diagnostics;
+using System.Runtime.Serialization;
 
 namespace System.Data
 {
@@ -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),
@@ -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++)
             {
@@ -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;
         }
 
@@ -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];

diff --git a/src/libraries/System.Data.Common/src/System/Data/LocalAppContextSwitches.cs b/src/libraries/System.Data.Common/src/System/Data/LocalAppContextSwitches.cs
@@ -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);
+        }
+    }
+}