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 appears to break usage of Dataset.xmlReader in SQL CLR routines #39706

Closed
WCLucas42 opened this issue Jul 21, 2020 · 13 comments
Assignees
Labels
area-Serialization tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly

Comments

@WCLucas42
Copy link

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" 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
Copy link
Collaborator

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.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jul 21, 2020
@blowdart
Copy link
Contributor

blowdart commented Jul 21, 2020

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

@blowdart blowdart added area-Serialization and removed untriaged New issue has not been triaged by the area owner labels Jul 21, 2020
@GrabYourPitchforks GrabYourPitchforks self-assigned this Jul 21, 2020
@GrabYourPitchforks
Copy link
Member

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

@WCLucas42
Copy link
Author

WCLucas42 commented Jul 21, 2020

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
Copy link
Contributor

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?

@blowdart blowdart added the tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly label Jul 21, 2020
@WCLucas42
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Member

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
Copy link
Author

WCLucas42 commented Jul 22, 2020 via email

@GrabYourPitchforks
Copy link
Member

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

@blowdart
Copy link
Contributor

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
Copy link

Monokel commented Aug 2, 2020

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

@dotnet dotnet locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Serialization tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly
Projects
None yet
Development

No branches or pull requests

6 participants