Disallow unrestricted polymorphic deserialization in DataSet #39304 appears to break usage of Dataset.xmlReader in SQL CLR routines

WCLucas42 commented 4 years ago

Description

After update a previously working CLR assembly now throws an error when called. System.Drawing is not a supported Library. https://docs.microsoft.com/en-us/sql/relational-databases/clr-integration/database-objects/supported-net-framework-libraries?view=sql-server-ver15

The Error is raised in the call to dataSet.ReadXml(xmlSR); in this block of code:

   // Add headers specified in the configuration data
    private static void AddRequestHeaders(WebClient webClient, string headers)
     {

        //verify some headers specified
        if (!String.IsNullOrEmpty(headers))
        {

            //convert xml to enumerable type and iterate
            StringReader xmlSR = new StringReader(headers);
            DataSet dataSet = new DataSet();
            dataSet.ReadXml(xmlSR, XmlReadMode.ReadSchema);
            DataTable dataTable = dataSet.Tables[0];

            foreach (DataRow row in dataTable.Rows)
            {
                //add header
                webClient.Headers[row["name"].ToString()] = row["value"].ToString();
            }
        }
    }

The error that is now presented is:

Msg 6522, Level 16, State 1, Procedure BMRAM.ClrRestClient_SendMessage, Line 0 [Batch Start Line 0] A .NET Framework error occurred during execution of user-defined routine or aggregate "ClrRestClient_SendMessage": System.TypeInitializationException: The type initializer for 'Scope' threw an exception. ---> System.IO.FileNotFoundException: Could not load file or assembly 'System.Drawing, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified. System.IO.FileNotFoundException: at System.Data.TypeLimiter.Scope..cctor() System.TypeInitializationException: at System.Data.TypeLimiter.Scope.IsTypeUnconditionallyAllowed(Type type) at System.Data.TypeLimiter.Scope.IsAllowedType(Type type) at System.Data.TypeLimiter.EnsureTypeIsAllowed(Type type, TypeLimiter capturedLimiter) at System.Data.DataColumn.UpdateColumnType(Type type, StorageType typeCode) at System.Data.DataColumn..ctor(String columnName, Type dataType, String expr, MappingType type) at System.Data.XSDSchema.HandleElementColumn(XmlSchemaElement elem, DataTable table, Boolean isBase) at System.Data.XSDSchema.HandleParticle(XmlSchemaParticle pt, DataTable table, ArrayList tableChildren, Boolean isBase) at System.Data.XSDSchema.HandleComplexType(XmlSchemaComplexType ct, DataTable table, ArrayList tableChildren, Boolean isNillable) at System.Data.XSDSchema.InstantiateTable(XmlSchemaElement node, XmlSchemaComplexType typeNode, Boolean isRef) at System.Data.XSDSchema.HandleTable(XmlSchemaElement node) at System.Data.XSDSchema.LoadSchema(XmlSchemaSet schemaSet, DataSet ds) at System.Data.DataSet.ReadXml(XmlReader reader, Boolean denyResolving) at ClrRestClient.RestClient.AddRequestHeaders(WebClient webClient, String headers) at ClrRestClient.RestClient.SendMessage(SqlString uriScheme, SqlString uriHost, SqlString uriPath, SqlInt32 uriPort, SqlString uriQuery, SqlString azureNamespace, SqlString topic, SqlString accessPolicyName, SqlString accessPolicyPrimaryKey, SqlInt64 tokenLifetime, SqlString verb, SqlString header...

Configuration

Regression?

Yes this CLR assembly has been working for over a year without issue

Other information

stack trace included in description. Running the assembly in its test rig still passes.
The relevant change set seems to be "Disallow unrestricted polymorphic deserialization in DataSet" #39304 This change set added TypeLimiter.cs
I had to factor out storing configured headers as xml relying on splitting strings. I couldn't add System.Drawing as an Assembly but that should work as well.

Dotnet-GitSync-Bot commented 4 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

blowdart commented 4 years ago

This is a deliberate change due to security issues, although it's not the error I'd expect. What exactly are you serializing from System.Drawing? Have you read the security guidance, including how to add more allowed types if you cannot move away from DataSet or DataTable?

@GrabYourPitchforks

GrabYourPitchforks commented 4 years ago

This is Full Framework servicing, not Core. I'll loop in the servicing team.

WCLucas42 commented 4 years ago

This is a deliberate change due to security issues, although it's not the error I'd expect. What exactly are you serializing from System.Drawing? Have you read the security guidance, including how to add more allowed types if you cannot move away from DataSet or DataTable?

@GrabYourPitchforks

First, I am not serializing anything from System.Drawing. See below for the current xml and presumed issue.

Second I, have not read the security guidance, I can do that to see if it presents and alternative, but I am not sure how pertinent it is to this issue, my xml only contains strings.

Finally, I did move away from datasets to a comma separated string of the format name1|value1, name2|value2 not an ideal change but it was quick and easy to push in the interim. I would like to return to a more standard format in the future.

Additional Assumptions

I am guessing the issue arises because System.Data became dependent on System.Drawing because of the its reference in TypeLimiter.cs.

using System.Collections.Generic;
using System.Data.SqlTypes;
using System.Diagnostics;
**using System.Drawing;**
using System.Linq;
using System.Numerics;
using System.Runtime.Serialization;

namespace System.Data
{
    internal sealed class TypeLimiter
    {
        [ThreadStatic]
        private static Scope? s_activeScope;

        private Scope m_instanceScope;

        private const string AppDomainDataSetDefaultAllowedTypesKey = "System.Data.DataSetDefaultAllowedTypes";

        private TypeLimiter(Scope scope)
        {
            Debug.Assert(scope != null);
            m_instanceScope = scope;
        }

The specific call in the stack refers to System.Data.TypeLimiter.Scope.IsTypeUnconditionallyAllowed(Type type)

It is referenced in private static readonly HashSet s_allowedTypes = new HashSet()

which is used in the above call.

So it appears any attempt to use XmlReader in CLR Routine will fail at this point without adding the assembly.

Associated XML to be serialized

<headers>
  <xsd:schema xmlns:schema="urn:schemas-microsoft-com:sql:SqlRowSet6" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:sqltypes="http://schemas.microsoft.com/sqlserver/2004/sqltypes" targetNamespace="urn:schemas-microsoft-com:sql:SqlRowSet6" elementFormDefault="qualified">
    <xsd:import namespace="http://schemas.microsoft.com/sqlserver/2004/sqltypes" schemaLocation="http://schemas.microsoft.com/sqlserver/2004/sqltypes/sqltypes.xsd" />
    <xsd:element name="header">
      <xsd:complexType>
        <xsd:sequence>
          <xsd:element name="name">
            <xsd:simpleType>
              <xsd:restriction base="sqltypes:nvarchar" sqltypes:localeId="1033" sqltypes:sqlCompareOptions="IgnoreCase IgnoreKanaType IgnoreWidth" sqltypes:sqlSortId="52">
                <xsd:maxLength value="128" />
              </xsd:restriction>
            </xsd:simpleType>
          </xsd:element>
          <xsd:element name="value" minOccurs="0">
            <xsd:simpleType>
              <xsd:restriction base="sqltypes:nvarchar" sqltypes:localeId="1033" sqltypes:sqlCompareOptions="IgnoreCase IgnoreKanaType IgnoreWidth" sqltypes:sqlSortId="52">
                <xsd:maxLength value="255" />
              </xsd:restriction>
            </xsd:simpleType>
          </xsd:element>
        </xsd:sequence>
      </xsd:complexType>
    </xsd:element>
  </xsd:schema>
  <header xmlns="urn:schemas-microsoft-com:sql:SqlRowSet6">
    <name>Accept-Encoding</name>
    <value>gzip, deflate</value>
  </header>
  <header xmlns="urn:schemas-microsoft-com:sql:SqlRowSet6">
    <name>Cache-Control</name>
    <value>no-cache</value>
  </header>
  <header xmlns="urn:schemas-microsoft-com:sql:SqlRowSet6">
    <name>ContentType</name>
    <value>application/json</value>
  </header>
</headers>

blowdart commented 4 years ago

OK. Frankly we never even thought of folks trying to use DataSet and DataTable XML functions in SQL CLR due to SQL having it's own XML parsing, and of course the CLR also having XML parsing.

As we safe listed some enums for System.Drawing in the patch for DataSet and DataTable that's where the exception is coming from, and there's not a lot you can do other than uninstall the patch, or adjust your code to take another approach. As this was a security patch we'd strongly suggest you choose the latter approach. Would XmlTextReader work in your situation?

WCLucas42 commented 4 years ago

Confirming this is intentional and persistent, I updated my code to just use an XML document and I have it working in our development environment. I would agree nobody in their right mind should be doing what I am doing here. So, other than a little grumbling this came down as a patch, this isn't an issue for me.

        private static void AddRequestHeaders(WebClient webClient, string headers)
        {

            if (!String.IsNullOrEmpty(headers))
            {
                StringReader xmlSR = new StringReader(headers);
                XmlDocument doc = new XmlDocument();
                doc.Load(xmlSR);
                XmlElement root = doc.DocumentElement;
                XmlNodeList nodes = root.SelectNodes("header");
                foreach (XmlNode node in nodes)
                {
                    webClient.Headers[node.SelectSingleNode("name").InnerText] = node.SelectSingleNode("value").InnerText;
                }
            }
        }

blowdart commented 4 years ago

Well now I'd never accuse customers of not being in a right mind. In public.

I'm glad you're up and running, so I'll close the issue (which didn't really apply, because this is for .net core, rather than framework), but I'm glad you opened it nevertheless because it's interesting to see how people are using dataset and datatable in unexpected ways.

WCLucas42 commented 4 years ago

Well now I'd never accuse customers of not being in a right mind. In public.

I'm glad you're up and running, so I'll close the issue (which didn't really apply, because this is for .net core, rather than framework), but I'm glad you opened it nevertheless because it's interesting to see how people are using dataset and datatable in unexpected ways.

In what little defense I can muster. The problematic code came from an MS supplied example.

And sorry about opening in the wrong repo this was the only reference I could find to the newly added call and I thought core made its way into all of the stuff at this point.

Thanks for the prompt replies.

akoeplinger commented 4 years ago

The problematic code came from an MS supplied example.

Would you mind linking to the example in case it is online? I assume we might want to take it down :)

WCLucas42 commented 4 years ago

I looked earlier, and checked my commit comments. No luck there… I will look a little more.

GrabYourPitchforks commented 4 years ago

(Ed. note: I scrubbed part of the most recent comment since it contained personal contact information. Apologies if this was inappropriate.)

blowdart commented 4 years ago

It was a Microsoft sample? That's even funnier :)

Don't worry about the wrong repo, SQL Clr doesn't have one, nor does .NET Framework.

In case security patches cause problems again you should know support calls for security patches and side effects are free, and you can call the support line to get things addressed.

Monokel commented 4 years ago

@WCLucas42 FYI, you are not the only one with this problem. We had the same problem, but we got it working again.

dotnet / runtime