[release/5.0-preview7] Disallow unrestricted polymorphic deserializat…

…ion in DataSet (#39314) 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 · 53976d3 · 53976d3
1 parent fe9e53e
commit 53976d3
Show file tree

Hide file tree

Showing 12 changed files with 824 additions and 13 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
@@ -2,7 +2,7 @@
   <PropertyGroup>
     <AssemblyName>System.Data.Common</AssemblyName>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
+    <TargetFrameworks>$(NetCoreAppCurrent)-Windows_NT;$(NetCoreAppCurrent)-Unix</TargetFrameworks>
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="System.Data.Common.TypeForwards.cs" />
@@ -123,6 +123,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" />
@@ -156,6 +160,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" />
@@ -295,25 +300,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
@@ -406,6 +406,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 typ
 
         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
@@ -1961,9 +1961,11 @@ private void WriteXmlSchema(XmlWriter writer, SchemaFormat schemaFormat, Convert
 
         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
                 {
@@ -2231,6 +2233,7 @@ internal XmlReadMode ReadXml(XmlReader reader, bool denyResolving)
             }
             finally
             {
+                restrictedScope?.Dispose();
                 DataCommonEventSource.Log.ExitScope(logScopeId);
             }
         }
@@ -2467,9 +2470,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)
@@ -2711,6 +2716,7 @@ internal XmlReadMode ReadXml(XmlReader reader, XmlReadMode mode, bool denyResolv
             }
             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
@@ -5659,9 +5659,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
                 {
@@ -5896,15 +5898,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;
@@ -6190,6 +6195,7 @@ internal XmlReadMode ReadXml(XmlReader reader, XmlReadMode mode, bool denyResolv
             }
             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
@@ -6,6 +6,7 @@
 using System.Data.Common;
 using System.Data.SqlTypes;
 using System.Diagnostics;
+using System.Runtime.Serialization;
 
 namespace System.Data
 {
@@ -16,6 +17,7 @@ internal sealed class FunctionNode : ExpressionNode
         internal int _argumentCount = 0;
         internal const int initialCapacity = 1;
         internal ExpressionNode[] _arguments;
+        private readonly TypeLimiter _capturedLimiter = null;
 
         private static readonly Function[] s_funcs = new Function[] {
             new Function("Abs", FunctionId.Abs, typeof(object), true, false, 1, typeof(object), null, null),
@@ -40,6 +42,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++)
             {
@@ -289,6 +297,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;
         }
 
@@ -494,10 +507,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,18 @@
+// 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.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);
+        }
+    }
+}