From 468b8a0c482be8935506d5bc50d0ff90a120a2bc Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Fri, 29 May 2026 15:56:50 -0700 Subject: [PATCH 1/4] Add a serialization binder for Service Fabric provider --- .../AllowedTypesSerializationBinderTests.cs | 192 ++++++++++++++++++ docs/providers/service-fabric.md | 20 ++ .../FabricOrchestrationProviderSettings.cs | 14 ++ .../AllowedTypesSerializationBinder.cs | 77 +++++++ .../Service/Startup.cs | 10 +- .../Service/TaskHubProxyListener.cs | 2 +- 6 files changed, 313 insertions(+), 2 deletions(-) create mode 100644 Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs create mode 100644 src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs diff --git a/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs b/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs new file mode 100644 index 000000000..c111a4318 --- /dev/null +++ b/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs @@ -0,0 +1,192 @@ +// ---------------------------------------------------------------------------------- +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +namespace DurableTask.AzureServiceFabric.Tests +{ + using System; + using System.Collections.Generic; + using DurableTask.AzureServiceFabric.Service; + using DurableTask.Core; + using DurableTask.Core.History; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using Newtonsoft.Json; + + [TestClass] + public class AllowedTypesSerializationBinderTests + { + readonly AllowedTypesSerializationBinder binder = new AllowedTypesSerializationBinder(); + + [TestMethod] + public void BindToType_AllowsDurableTaskCoreTypes() + { + var type = typeof(TaskMessage); + var result = this.binder.BindToType(type.Assembly.GetName().Name, type.FullName); + Assert.AreEqual(type, result); + } + + [TestMethod] + public void BindToType_AllowsHistoryEventSubclasses() + { + var type = typeof(ExecutionStartedEvent); + var result = this.binder.BindToType(type.Assembly.GetName().Name, type.FullName); + Assert.AreEqual(type, result); + } + + [TestMethod] + public void BindToType_AllowsServiceFabricTypes() + { + var type = typeof(FabricOrchestrationProvider); + var result = this.binder.BindToType(type.Assembly.GetName().Name, type.FullName); + Assert.AreEqual(type, result); + } + + [TestMethod] + public void BindToType_AllowsMscorlibTypes() + { + var type = typeof(Dictionary); + var result = this.binder.BindToType("mscorlib", type.FullName); + Assert.AreEqual(type, result); + } + + [TestMethod] + public void BindToType_AllowsQualifiedAssemblyName() + { + var type = typeof(TaskMessage); + string qualifiedName = type.Assembly.FullName; // e.g. "DurableTask.Core, Version=..." + var result = this.binder.BindToType(qualifiedName, type.FullName); + Assert.AreEqual(type, result); + } + + [TestMethod] + public void BindToType_AllowsNullAssemblyName() + { + // Null/empty assembly name should pass through to the default binder + var result = this.binder.BindToType(null, typeof(string).FullName); + Assert.IsNotNull(result); + } + + [TestMethod] + public void BindToType_RejectsArbitraryAssembly() + { + Assert.ThrowsException(() => + this.binder.BindToType("Evil.Assembly", "Evil.PwnedDescriptor")); + } + + [TestMethod] + public void BindToType_RejectsQualifiedArbitraryAssembly() + { + Assert.ThrowsException(() => + this.binder.BindToType("Evil.Assembly, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null", "Evil.PwnedDescriptor")); + } + + [TestMethod] + public void BindToType_RejectsSystemDiagnosticsProcess() + { + // A common gadget type — must be rejected + var type = typeof(System.Diagnostics.Process); + Assert.ThrowsException(() => + this.binder.BindToType(type.Assembly.GetName().Name, type.FullName)); + } + + [TestMethod] + public void BindToName_DelegatesToDefaultBinder() + { + this.binder.BindToName(typeof(TaskMessage), out string assemblyName, out string typeName); + // DefaultSerializationBinder delegates to the runtime; just verify it doesn't throw + // and returns consistent results for a known type. + this.binder.BindToName(typeof(TaskMessage), out string assemblyName2, out string typeName2); + Assert.AreEqual(assemblyName, assemblyName2); + Assert.AreEqual(typeName, typeName2); + } + + [TestMethod] + public void RoundTrip_TaskMessageWithHistoryEvent_Succeeds() + { + var message = new TaskMessage + { + SequenceNumber = 42, + OrchestrationInstance = new OrchestrationInstance { InstanceId = "test-1", ExecutionId = "exec-1" }, + Event = new ExecutionStartedEvent(-1, "input-data") + { + Tags = new Dictionary { { "key", "value" } }, + Name = "TestOrchestration", + Version = "1.0" + } + }; + + var settings = new JsonSerializerSettings + { + TypeNameHandling = TypeNameHandling.All, + SerializationBinder = this.binder + }; + + string json = JsonConvert.SerializeObject(message, settings); + var deserialized = JsonConvert.DeserializeObject(json, settings); + + Assert.IsNotNull(deserialized); + Assert.AreEqual(42, deserialized.SequenceNumber); + Assert.AreEqual("test-1", deserialized.OrchestrationInstance.InstanceId); + Assert.IsInstanceOfType(deserialized.Event, typeof(ExecutionStartedEvent)); + + var startedEvent = (ExecutionStartedEvent)deserialized.Event; + Assert.AreEqual("TestOrchestration", startedEvent.Name); + Assert.AreEqual("value", startedEvent.Tags["key"]); + } + + [TestMethod] + public void Deserialize_MaliciousPayload_IsRejected() + { + string maliciousJson = @"{ + ""$type"": ""System.Diagnostics.Process, System"", + ""StartInfo"": { ""FileName"": ""cmd.exe"" } + }"; + + var settings = new JsonSerializerSettings + { + TypeNameHandling = TypeNameHandling.All, + SerializationBinder = this.binder + }; + + // Newtonsoft wraps the binder's InvalidOperationException in a JsonSerializationException + var ex = Assert.ThrowsException(() => + JsonConvert.DeserializeObject(maliciousJson, settings)); + Assert.IsInstanceOfType(ex.InnerException, typeof(InvalidOperationException)); + StringAssert.Contains(ex.InnerException.Message, "is not allowed"); + } + + [TestMethod] + public void Settings_DefaultBinderIsAllowedTypes() + { + var providerSettings = new FabricOrchestrationProviderSettings(); + Assert.IsNotNull(providerSettings.JsonSerializationBinder); + Assert.IsInstanceOfType(providerSettings.JsonSerializationBinder, typeof(AllowedTypesSerializationBinder)); + } + + [TestMethod] + public void Settings_BinderCanBeSetToNull() + { + var providerSettings = new FabricOrchestrationProviderSettings(); + providerSettings.JsonSerializationBinder = null; + Assert.IsNull(providerSettings.JsonSerializationBinder); + } + + [TestMethod] + public void Settings_BinderCanBeOverridden() + { + var customBinder = new Newtonsoft.Json.Serialization.DefaultSerializationBinder(); + var providerSettings = new FabricOrchestrationProviderSettings(); + providerSettings.JsonSerializationBinder = customBinder; + Assert.AreSame(customBinder, providerSettings.JsonSerializationBinder); + } + } +} diff --git a/docs/providers/service-fabric.md b/docs/providers/service-fabric.md index 5109d288f..0bdc54c8f 100644 --- a/docs/providers/service-fabric.md +++ b/docs/providers/service-fabric.md @@ -131,6 +131,7 @@ Service Fabric handles partitioning automatically based on your service configur | `TaskActivityDispatcherSettings.MaxConcurrentActivities` | Max concurrent activities | 1000 | | `TaskActivityDispatcherSettings.DispatcherCount` | Number of activity dispatchers | 10 | | `LoggerFactory` | Optional logger factory for diagnostics | null | +| `JsonSerializationBinder` | `ISerializationBinder` that restricts which types can be deserialized from incoming JSON requests on the proxy endpoint | `AllowedTypesSerializationBinder` | ### Example Configuration @@ -150,6 +151,25 @@ var settings = new FabricOrchestrationProviderSettings }; ``` +### Serialization Security + +The proxy endpoint uses `TypeNameHandling.All` for JSON deserialization to support polymorphic types like `HistoryEvent`. By default, an `AllowedTypesSerializationBinder` restricts deserialization to types from `DurableTask.Core`, `DurableTask.AzureServiceFabric`, and core system assemblies. This prevents untrusted `$type` metadata in JSON payloads from loading arbitrary types. + +To provide a custom binder: + +```csharp +settings.JsonSerializationBinder = new MyCustomSerializationBinder(); +``` + +To disable type restrictions and restore legacy behavior: + +```csharp +// ⚠️ Not recommended: disables deserialization type restrictions. +// Only use this if you have other security controls in place +// (e.g., network isolation, mutual TLS) to protect the proxy endpoint. +settings.JsonSerializationBinder = null; +``` + ## Client Access ### From Within Service Fabric diff --git a/src/DurableTask.AzureServiceFabric/FabricOrchestrationProviderSettings.cs b/src/DurableTask.AzureServiceFabric/FabricOrchestrationProviderSettings.cs index 359b69ac9..41a78bf91 100644 --- a/src/DurableTask.AzureServiceFabric/FabricOrchestrationProviderSettings.cs +++ b/src/DurableTask.AzureServiceFabric/FabricOrchestrationProviderSettings.cs @@ -13,9 +13,11 @@ namespace DurableTask.AzureServiceFabric { + using DurableTask.AzureServiceFabric.Service; using DurableTask.Core; using DurableTask.Core.Settings; using Microsoft.Extensions.Logging; + using Newtonsoft.Json.Serialization; /// /// Provides settings for service fabric based custom provider implementations @@ -56,5 +58,17 @@ public FabricOrchestrationProviderSettings() /// Gets or sets the optional to use for diagnostic logging. /// public ILoggerFactory LoggerFactory { get; set; } + + /// + /// Gets or sets the used to restrict which types can be + /// deserialized from incoming JSON requests on the proxy endpoint. This protects against + /// untrusted $type metadata in JSON payloads. + /// + /// Defaults to , which permits only known + /// DurableTask and core system types. Set a custom to + /// override, or set to null to disable type restrictions (not recommended). + /// + /// + public ISerializationBinder JsonSerializationBinder { get; set; } = new AllowedTypesSerializationBinder(); } } diff --git a/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs new file mode 100644 index 000000000..2e6aeb6e9 --- /dev/null +++ b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs @@ -0,0 +1,77 @@ +// ---------------------------------------------------------------------------------- +// Copyright Microsoft Corporation +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// http://www.apache.org/licenses/LICENSE-2.0 +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// ---------------------------------------------------------------------------------- + +namespace DurableTask.AzureServiceFabric.Service +{ + using System; + using System.Collections.Concurrent; + using System.Collections.Generic; + using System.Linq; + using System.Reflection; + using Newtonsoft.Json.Serialization; + + /// + /// A serialization binder that restricts deserialization to only known DurableTask types + /// and core system types. This prevents untrusted $type metadata in JSON payloads + /// from loading arbitrary assemblies or instantiating arbitrary types. + /// + public sealed class AllowedTypesSerializationBinder : ISerializationBinder + { + static readonly HashSet AllowedAssemblyNames = new HashSet(StringComparer.OrdinalIgnoreCase) + { + typeof(Core.TaskMessage).Assembly.GetName().Name, // DurableTask.Core + typeof(FabricOrchestrationProvider).Assembly.GetName().Name, // DurableTask.AzureServiceFabric + "mscorlib", // .NET Framework core types + "System.Private.CoreLib", // .NET Core/5+ core types + }; + + readonly DefaultSerializationBinder defaultBinder = new DefaultSerializationBinder(); + readonly ConcurrentDictionary assemblyAllowCache = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + + /// + public Type BindToType(string assemblyName, string typeName) + { + if (!IsAssemblyAllowed(assemblyName)) + { + throw new InvalidOperationException( + $"Deserialization of type '{typeName}' from assembly '{assemblyName}' is not allowed. " + + $"Only known DurableTask and core system types are permitted."); + } + + return this.defaultBinder.BindToType(assemblyName, typeName); + } + + /// + public void BindToName(Type serializedType, out string assemblyName, out string typeName) + { + this.defaultBinder.BindToName(serializedType, out assemblyName, out typeName); + } + + bool IsAssemblyAllowed(string assemblyName) + { + if (string.IsNullOrWhiteSpace(assemblyName)) + { + // No assembly specified — let the default binder resolve it. + return true; + } + + return this.assemblyAllowCache.GetOrAdd(assemblyName, name => + { + // Strip version/culture/publicKeyToken if present (e.g., "mscorlib, Version=4.0.0.0, ...") + int commaIndex = name.IndexOf(','); + string shortName = commaIndex >= 0 ? name.Substring(0, commaIndex).Trim() : name.Trim(); + return AllowedAssemblyNames.Contains(shortName); + }); + } + } +} diff --git a/src/DurableTask.AzureServiceFabric/Service/Startup.cs b/src/DurableTask.AzureServiceFabric/Service/Startup.cs index f9e0beac7..a96ecd18e 100644 --- a/src/DurableTask.AzureServiceFabric/Service/Startup.cs +++ b/src/DurableTask.AzureServiceFabric/Service/Startup.cs @@ -21,17 +21,20 @@ namespace DurableTask.AzureServiceFabric.Service using DurableTask.Core; using DurableTask.AzureServiceFabric; using Microsoft.Extensions.DependencyInjection; + using Newtonsoft.Json.Serialization; using Owin; class Startup : IOwinAppBuilder { FabricOrchestrationProvider fabricOrchestrationProvider; + ISerializationBinder serializationBinder; string listeningAddress; - public Startup(string listeningAddress, FabricOrchestrationProvider fabricOrchestrationProvider) + public Startup(string listeningAddress, FabricOrchestrationProvider fabricOrchestrationProvider, ISerializationBinder serializationBinder) { this.listeningAddress = listeningAddress ?? throw new ArgumentNullException(nameof(listeningAddress)); this.fabricOrchestrationProvider = fabricOrchestrationProvider ?? throw new ArgumentNullException(nameof(fabricOrchestrationProvider)); + this.serializationBinder = serializationBinder; } public string GetListeningAddress() @@ -61,6 +64,11 @@ void IOwinAppBuilder.Startup(IAppBuilder appBuilder) config.Formatters.Remove(config.Formatters.XmlFormatter); config.Formatters.Remove(config.Formatters.FormUrlEncodedFormatter); config.Formatters.JsonFormatter.SerializerSettings.TypeNameHandling = Newtonsoft.Json.TypeNameHandling.All; + if (this.serializationBinder != null) + { + config.Formatters.JsonFormatter.SerializerSettings.SerializationBinder = this.serializationBinder; + } + appBuilder.UseWebApi(config); } } diff --git a/src/DurableTask.AzureServiceFabric/Service/TaskHubProxyListener.cs b/src/DurableTask.AzureServiceFabric/Service/TaskHubProxyListener.cs index 86aba90f2..03fcb22ce 100644 --- a/src/DurableTask.AzureServiceFabric/Service/TaskHubProxyListener.cs +++ b/src/DurableTask.AzureServiceFabric/Service/TaskHubProxyListener.cs @@ -163,7 +163,7 @@ public ServiceReplicaListener CreateServiceReplicaListener() string protocol = this.enableHttps ? "https" : "http"; string listeningAddress = string.Format(CultureInfo.InvariantCulture, "{0}://{1}:{2}/{3}/dtfx/", protocol, ipAddress, serviceEndpoint.Port, context.PartitionId); - return new OwinCommunicationListener(new Startup(listeningAddress, this.fabricOrchestrationProvider)); + return new OwinCommunicationListener(new Startup(listeningAddress, this.fabricOrchestrationProvider, this.fabricOrchestrationProviderSettings.JsonSerializationBinder)); }, Constants.TaskHubProxyServiceName); } From 81e1ca00d75a6e3b88b55b9169337380016a6bf8 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Fri, 29 May 2026 16:08:49 -0700 Subject: [PATCH 2/4] Enhance AllowedTypesSerializationBinder to restrict deserialization to specific Service Fabric proxy types and improve type validation logic --- .../AllowedTypesSerializationBinderTests.cs | 45 +++++- .../AllowedTypesSerializationBinder.cs | 131 +++++++++++++++--- 2 files changed, 151 insertions(+), 25 deletions(-) diff --git a/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs b/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs index c111a4318..ccc02c1f0 100644 --- a/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs +++ b/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs @@ -15,6 +15,9 @@ namespace DurableTask.AzureServiceFabric.Tests { using System; using System.Collections.Generic; + using System.Linq; + using System.Reflection; + using DurableTask.AzureServiceFabric.Models; using DurableTask.AzureServiceFabric.Service; using DurableTask.Core; using DurableTask.Core.History; @@ -43,9 +46,9 @@ public void BindToType_AllowsHistoryEventSubclasses() } [TestMethod] - public void BindToType_AllowsServiceFabricTypes() + public void BindToType_AllowsServiceFabricProxyTypes() { - var type = typeof(FabricOrchestrationProvider); + var type = typeof(CreateTaskOrchestrationParameters); var result = this.binder.BindToType(type.Assembly.GetName().Name, type.FullName); Assert.AreEqual(type, result); } @@ -98,6 +101,44 @@ public void BindToType_RejectsSystemDiagnosticsProcess() this.binder.BindToType(type.Assembly.GetName().Name, type.FullName)); } + [TestMethod] + public void BindToType_RejectsNonAllowlistedMscorlibType() + { + // System.Type is from mscorlib but not in the type allowlist + Assert.ThrowsException(() => + this.binder.BindToType("mscorlib", typeof(Type).FullName)); + } + + [TestMethod] + public void BindToType_RejectsNonAllowlistedDurableTaskCoreType() + { + // TaskOrchestration is a DurableTask.Core type but not in the proxy endpoint allowlist + var type = typeof(TaskOrchestration); + Assert.ThrowsException(() => + this.binder.BindToType(type.Assembly.GetName().Name, type.FullName)); + } + + [TestMethod] + public void BindToType_AllowsAllHistoryEventKnownTypes() + { + IEnumerable knownTypes; + try + { + knownTypes = HistoryEvent.KnownTypes(); + } + catch (ReflectionTypeLoadException ex) + { + // In test environments, not all types may be loadable + knownTypes = ex.Types.Where(t => t != null && !t.IsAbstract && typeof(HistoryEvent).IsAssignableFrom(t)); + } + + foreach (Type knownType in knownTypes) + { + var result = this.binder.BindToType(knownType.Assembly.GetName().Name, knownType.FullName); + Assert.AreEqual(knownType, result, $"HistoryEvent subclass {knownType.Name} should be allowed"); + } + } + [TestMethod] public void BindToName_DelegatesToDefaultBinder() { diff --git a/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs index 2e6aeb6e9..67be5deff 100644 --- a/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs +++ b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs @@ -18,37 +18,46 @@ namespace DurableTask.AzureServiceFabric.Service using System.Collections.Generic; using System.Linq; using System.Reflection; + using DurableTask.AzureServiceFabric.Models; + using DurableTask.Core; + using DurableTask.Core.History; + using DurableTask.Core.Tracing; using Newtonsoft.Json.Serialization; /// - /// A serialization binder that restricts deserialization to only known DurableTask types - /// and core system types. This prevents untrusted $type metadata in JSON payloads - /// from loading arbitrary assemblies or instantiating arbitrary types. + /// A serialization binder that restricts deserialization to only the specific types + /// that flow through the Service Fabric proxy HTTP endpoints. This prevents untrusted + /// $type metadata in JSON payloads from loading arbitrary assemblies or + /// instantiating arbitrary types. /// public sealed class AllowedTypesSerializationBinder : ISerializationBinder { - static readonly HashSet AllowedAssemblyNames = new HashSet(StringComparer.OrdinalIgnoreCase) - { - typeof(Core.TaskMessage).Assembly.GetName().Name, // DurableTask.Core - typeof(FabricOrchestrationProvider).Assembly.GetName().Name, // DurableTask.AzureServiceFabric - "mscorlib", // .NET Framework core types - "System.Private.CoreLib", // .NET Core/5+ core types - }; + static readonly HashSet AllowedTypes = BuildAllowedTypes(); readonly DefaultSerializationBinder defaultBinder = new DefaultSerializationBinder(); - readonly ConcurrentDictionary assemblyAllowCache = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + readonly ConcurrentDictionary typeAllowCache = new ConcurrentDictionary(); /// public Type BindToType(string assemblyName, string typeName) { - if (!IsAssemblyAllowed(assemblyName)) + // Validate assembly name before resolving to prevent Assembly.Load of untrusted assemblies. + if (!string.IsNullOrWhiteSpace(assemblyName) && !IsAssemblyNameAllowed(assemblyName)) { throw new InvalidOperationException( $"Deserialization of type '{typeName}' from assembly '{assemblyName}' is not allowed. " + - $"Only known DurableTask and core system types are permitted."); + $"Only known DurableTask proxy endpoint types are permitted."); + } + + Type resolvedType = this.defaultBinder.BindToType(assemblyName, typeName); + + if (!IsTypeAllowed(resolvedType)) + { + throw new InvalidOperationException( + $"Deserialization of type '{resolvedType.FullName}' is not allowed. " + + $"Only known DurableTask proxy endpoint types are permitted."); } - return this.defaultBinder.BindToType(assemblyName, typeName); + return resolvedType; } /// @@ -57,20 +66,96 @@ public void BindToName(Type serializedType, out string assemblyName, out string this.defaultBinder.BindToName(serializedType, out assemblyName, out typeName); } - bool IsAssemblyAllowed(string assemblyName) + static HashSet BuildAllowedTypes() { - if (string.IsNullOrWhiteSpace(assemblyName)) + var types = new HashSet + { + // Core domain types that appear in the proxy endpoint object graph + typeof(TaskMessage), + typeof(OrchestrationInstance), + typeof(OrchestrationExecutionContext), + typeof(OrchestrationState), + typeof(ParentInstance), + typeof(DistributedTraceContext), + typeof(FailureDetails), + + // Service Fabric proxy parameter types + typeof(CreateTaskOrchestrationParameters), + typeof(PurgeOrchestrationHistoryParameters), + + // System collections used for Tags and FailureDetails.Properties + typeof(Dictionary), + typeof(Dictionary), + }; + + // All HistoryEvent subclasses (self-maintaining via assembly scanning). + // Use the same logic as HistoryEvent.KnownTypes() but handle partial load failures + // that can occur when not all referenced assemblies are available. + try + { + foreach (Type historyEventType in HistoryEvent.KnownTypes()) + { + types.Add(historyEventType); + } + } + catch (ReflectionTypeLoadException ex) { - // No assembly specified — let the default binder resolve it. - return true; + foreach (Type loadedType in ex.Types) + { + if (loadedType != null && !loadedType.IsAbstract && typeof(HistoryEvent).IsAssignableFrom(loadedType)) + { + types.Add(loadedType); + } + } } - return this.assemblyAllowCache.GetOrAdd(assemblyName, name => + return types; + } + + static bool IsAssemblyNameAllowed(string assemblyName) + { + // Strip version/culture/publicKeyToken if present + int commaIndex = assemblyName.IndexOf(','); + string shortName = commaIndex >= 0 ? assemblyName.Substring(0, commaIndex).Trim() : assemblyName.Trim(); + + return string.Equals(shortName, typeof(TaskMessage).Assembly.GetName().Name, StringComparison.OrdinalIgnoreCase) + || string.Equals(shortName, typeof(FabricOrchestrationProvider).Assembly.GetName().Name, StringComparison.OrdinalIgnoreCase) + || string.Equals(shortName, "mscorlib", StringComparison.OrdinalIgnoreCase) + || string.Equals(shortName, "System.Private.CoreLib", StringComparison.OrdinalIgnoreCase); + } + + bool IsTypeAllowed(Type type) + { + return this.typeAllowCache.GetOrAdd(type, t => { - // Strip version/culture/publicKeyToken if present (e.g., "mscorlib, Version=4.0.0.0, ...") - int commaIndex = name.IndexOf(','); - string shortName = commaIndex >= 0 ? name.Substring(0, commaIndex).Trim() : name.Trim(); - return AllowedAssemblyNames.Contains(shortName); + // Primitives and strings are always safe + if (t.IsPrimitive || t == typeof(string) || t == typeof(DateTime) + || t == typeof(DateTimeOffset) || t == typeof(TimeSpan) + || t == typeof(Guid) || t == typeof(decimal)) + { + return true; + } + + // Arrays of allowed types + if (t.IsArray) + { + return IsTypeAllowed(t.GetElementType()); + } + + // Nullable of allowed types + Type nullable = Nullable.GetUnderlyingType(t); + if (nullable != null) + { + return IsTypeAllowed(nullable); + } + + // Enums are safe (serialize as values) + if (t.IsEnum) + { + return true; + } + + return AllowedTypes.Contains(t); }); } } From 31b32b476dc0c2d4fce427cf162bca5ecd4c3dbe Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Mon, 1 Jun 2026 15:31:53 -0700 Subject: [PATCH 3/4] Address Copilot's comments --- .../Service/AllowedTypesSerializationBinder.cs | 7 ++----- src/DurableTask.AzureServiceFabric/Service/Startup.cs | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs index 67be5deff..13b5bcf9a 100644 --- a/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs +++ b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs @@ -100,12 +100,9 @@ static HashSet BuildAllowedTypes() } catch (ReflectionTypeLoadException ex) { - foreach (Type loadedType in ex.Types) + foreach (Type loadedType in ex.Types.Where(t => t != null && !t.IsAbstract && typeof(HistoryEvent).IsAssignableFrom(t))) { - if (loadedType != null && !loadedType.IsAbstract && typeof(HistoryEvent).IsAssignableFrom(loadedType)) - { - types.Add(loadedType); - } + types.Add(loadedType); } } diff --git a/src/DurableTask.AzureServiceFabric/Service/Startup.cs b/src/DurableTask.AzureServiceFabric/Service/Startup.cs index a96ecd18e..ce676d523 100644 --- a/src/DurableTask.AzureServiceFabric/Service/Startup.cs +++ b/src/DurableTask.AzureServiceFabric/Service/Startup.cs @@ -27,7 +27,7 @@ namespace DurableTask.AzureServiceFabric.Service class Startup : IOwinAppBuilder { FabricOrchestrationProvider fabricOrchestrationProvider; - ISerializationBinder serializationBinder; + readonly ISerializationBinder serializationBinder; string listeningAddress; public Startup(string listeningAddress, FabricOrchestrationProvider fabricOrchestrationProvider, ISerializationBinder serializationBinder) From 350a44db993f03ab752c65b2327f4253bc98d8d1 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Mon, 1 Jun 2026 15:43:27 -0700 Subject: [PATCH 4/4] Add test for unresolvable type in AllowedTypesSerializationBinder and improve error message --- .../AllowedTypesSerializationBinderTests.cs | 9 +++++++++ .../Service/AllowedTypesSerializationBinder.cs | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs b/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs index ccc02c1f0..8e6f58822 100644 --- a/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs +++ b/Test/DurableTask.AzureServiceFabric.Tests/AllowedTypesSerializationBinderTests.cs @@ -109,6 +109,15 @@ public void BindToType_RejectsNonAllowlistedMscorlibType() this.binder.BindToType("mscorlib", typeof(Type).FullName)); } + [TestMethod] + public void BindToType_RejectsUnresolvableType() + { + // A type name that cannot be resolved should throw a controlled exception, not NullReferenceException + var ex = Assert.ThrowsException(() => + this.binder.BindToType("DurableTask.Core", "DurableTask.Core.NonExistentType")); + StringAssert.Contains(ex.Message, "NonExistentType"); + } + [TestMethod] public void BindToType_RejectsNonAllowlistedDurableTaskCoreType() { diff --git a/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs index 13b5bcf9a..7507d9d15 100644 --- a/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs +++ b/src/DurableTask.AzureServiceFabric/Service/AllowedTypesSerializationBinder.cs @@ -50,10 +50,10 @@ public Type BindToType(string assemblyName, string typeName) Type resolvedType = this.defaultBinder.BindToType(assemblyName, typeName); - if (!IsTypeAllowed(resolvedType)) + if (resolvedType == null || !IsTypeAllowed(resolvedType)) { throw new InvalidOperationException( - $"Deserialization of type '{resolvedType.FullName}' is not allowed. " + + $"Deserialization of type '{typeName}' from assembly '{assemblyName}' is not allowed. " + $"Only known DurableTask proxy endpoint types are permitted."); }