-
Notifications
You must be signed in to change notification settings - Fork 336
Harden UDT assembly loading against server-supplied assembly names #4591
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
base: main
Are you sure you want to change the base?
Changes from all commits
c429b2f
5552db7
cd79651
c191b74
a3f1846
ff52061
421f56e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -257,6 +257,78 @@ AppContext switches allow runtime behavior changes without modifying connection | |
| | `Switch.Microsoft.Data.SqlClient.UseConnectionPoolV2` | `false` | Enables the new `ChannelDbConnectionPool` implementation | | ||
| | `Switch.Microsoft.Data.SqlClient.UseManagedNetworkingOnWindows` | `false` | Forces managed SNI on Windows (instead of native SNI) | | ||
| | `Switch.Microsoft.Data.SqlClient.UseOneSecFloorInTimeoutCalculationDuringLogin` | `false` | Sets 1-second minimum in login timeout calculations | | ||
| | `Switch.Microsoft.Data.SqlClient.UseLegacyUdtAssemblyLoad` | `false` | Restores the pre-policy behavior of loading any assembly named by a server-supplied UDT assembly-qualified name, and of skipping the `[SqlUserDefinedType]` check | | ||
|
|
||
| ### UDT Assembly Load Policy | ||
|
|
||
| A server-supplied UDT assembly-qualified name reaches `Assembly.Load`, so the | ||
| driver applies a deny-by-default policy before handing the name to the loader. | ||
| There is a single enforcing behavior, which permits: | ||
|
|
||
| | Permitted | Notes | | ||
| |-----------|-------| | ||
| | `Microsoft.SqlServer.Types` | Identity pinned: the version is normalized to the connection's negotiated type system version, and the public key token to the one Microsoft signs with | | ||
| | Assemblies on the allow list | The application explicitly naming what it is willing to have loaded | | ||
| | Assemblies already loaded into the process | Resolved to the instance the process already holds; the server-supplied version and public key token are discarded | | ||
|
|
||
| Everything else is refused. In particular, an assembly that is only *statically | ||
| referenced* by a loaded assembly is **not** permitted, because loading it is a | ||
| genuinely new load — precisely what this policy keeps under the application's | ||
| control rather than the server's. | ||
|
|
||
| Setting `UseLegacyUdtAssemblyLoad` disables the policy entirely and restores the | ||
| pre-policy behavior. It is a temporary compatibility escape hatch, not a | ||
| supported configuration. | ||
|
|
||
| Applications that use custom UDTs whose assemblies are loaded on demand must name | ||
| them explicitly through the `Microsoft.Data.SqlClient.UdtAssemblyAllowList` | ||
| AppContext data element, a semicolon-separated list of assembly names: | ||
|
|
||
| ```csharp | ||
| AppDomain.CurrentDomain.SetData( | ||
| "Microsoft.Data.SqlClient.UdtAssemblyAllowList", | ||
| "Contoso.Udts;Fabrikam.Udts, Version=2.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a"); | ||
| ``` | ||
|
|
||
| Each entry is matched only on the components it specifies, so a simple name | ||
| permits any version, culture, and public key token, while a fully-qualified name | ||
| must match exactly. | ||
|
|
||
| Independently of the assembly policy, a resolved type that is not annotated with | ||
| `SqlUserDefinedTypeAttribute` is rejected before any of its code runs (except | ||
| under `UseLegacyUdtAssemblyLoad`). This is the gate that actually prevents | ||
| foreign code execution: on CoreCLR, neither `Assembly.Load`, nor resolving a type | ||
| from the assembly, nor reading that type's custom attributes runs anything from | ||
| it — a module initializer or static constructor runs on first real member access, | ||
| which is what `GetUdtValue` would otherwise perform. | ||
|
|
||
| #### Compatibility impact | ||
|
|
||
| This policy is a behavior change for applications that use **custom** UDTs. The | ||
| built-in spatial types (`SqlGeography`, `SqlGeometry`, `SqlHierarchyId`) are | ||
| unaffected, since `Microsoft.SqlServer.Types` is permitted by identity. | ||
|
|
||
| An application is affected when the custom UDT's assembly is not yet loaded at | ||
| the moment the value is read. That is common whenever the *driver* materializes | ||
| the value and the application never names the type in its own code — generic data | ||
| access layers, micro-ORMs, `DataTable.Load`, and schema discovery. In those cases | ||
| the driver's own `Assembly.Load` was previously the thing that pulled the | ||
| assembly in, and it is now refused. | ||
|
|
||
| The symptom depends on the API: | ||
|
|
||
| | API | Symptom | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Flagging a few other scenarios.
I think most of them would be permitted: they either involve us transmitting UDTs, or us transferring them as a byte array without interpretation. |
||
| |-----|---------| | ||
| | `reader[i]`, `GetValue`, UDT output parameters | `SqlException` naming the assembly and the allow list | | ||
|
|
||
| | `GetFieldType`, `GetSchemaTable`, `GetColumnSchema` | Returns `null` for the UDT column's type rather than throwing | | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another possibility might be to have a new public type, If so, clients sometimes use Activator.CreateInstance on the type. In such cases, having the default ctor throw would be a reasonably simple point of contact for them. |
||
|
|
||
| The second row is the harder one to diagnose, because `GetFieldType` does not | ||
| normally return `null`; a caller that dereferences the result sees an unrelated | ||
| `NullReferenceException`. A denial is always traced through | ||
| `SqlClientEventSource` regardless of which path was taken, so enabling event | ||
| source tracing will identify the assembly. | ||
|
|
||
| The remedy in every case is to name the assembly on the allow list. | ||
|
|
||
| ### Usage Example | ||
| ```csharp | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If foreign code execution is the primary concern then
GetCustomAttributeswill run the attributes' constructors and module initializers. CustomAttributeData may be more relevant for our use case.