From b168f203159781582d724e911d23dcc7906b3045 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Thu, 30 Apr 2026 16:41:47 -0700 Subject: [PATCH 1/5] Add HistoryEventSerializationBinder to constrain HistoryEvent deserialization Mitigates https://msazure.visualstudio.com/Antares/_workitems/edit/37181649 --- .../HistoryEventSerializationBinderTests.cs | 113 ++++++++++++++++++ ...ureTableOrchestrationHistoryEventEntity.cs | 3 +- .../HistoryEventSerializationBinder.cs | 67 +++++++++++ 3 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs create mode 100644 src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs diff --git a/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs b/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs new file mode 100644 index 000000000..7acb84313 --- /dev/null +++ b/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs @@ -0,0 +1,113 @@ +// ---------------------------------------------------------------------------------- +// 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.ServiceBus.Tests +{ + using System; + using System.Collections.Generic; + using DurableTask.Core; + using DurableTask.Core.History; + using DurableTask.ServiceBus.Tracking; + using Microsoft.VisualStudio.TestTools.UnitTesting; + using Newtonsoft.Json; + + [TestClass] + public class HistoryEventSerializationBinderTests + { + // Mirrors the production read settings on AzureTableOrchestrationHistoryEventEntity. + static readonly JsonSerializerSettings ReadJsonSettings = new JsonSerializerSettings + { + TypeNameHandling = TypeNameHandling.Objects, + SerializationBinder = new HistoryEventSerializationBinder() + }; + + // Settings used to *produce* the JSON exactly as the production code does. + static readonly JsonSerializerSettings WriteJsonSettings = new JsonSerializerSettings + { + TypeNameHandling = TypeNameHandling.Objects + }; + + [TestMethod] + public void RoundTripsExecutionStartedEventWithTags() + { + var original = new ExecutionStartedEvent(eventId: -1, input: "input") + { + OrchestrationInstance = new OrchestrationInstance + { + InstanceId = "instance-1", + ExecutionId = "execution-1", + }, + Name = "OrchestrationName", + Version = "1.0", + Tags = new Dictionary { ["tag1"] = "value1" }, + }; + + string json = JsonConvert.SerializeObject(original, WriteJsonSettings); + var deserialized = JsonConvert.DeserializeObject(json, ReadJsonSettings); + + Assert.IsInstanceOfType(deserialized, typeof(ExecutionStartedEvent)); + var deserializedStarted = (ExecutionStartedEvent)deserialized; + Assert.AreEqual("OrchestrationName", deserializedStarted.Name); + Assert.AreEqual("input", deserializedStarted.Input); + Assert.AreEqual("value1", deserializedStarted.Tags["tag1"]); + } + + [TestMethod] + public void AllowsAllConcreteHistoryEventTypes() + { + // Sanity check that every concrete HistoryEvent subclass declared in DurableTask.Core + // is accepted by the binder's BindToType. + var binder = new HistoryEventSerializationBinder(); + foreach (Type concreteType in HistoryEvent.KnownTypes()) + { + Type bound = binder.BindToType(concreteType.Assembly.GetName().Name, concreteType.FullName); + Assert.AreEqual(concreteType, bound); + } + } + + [TestMethod] + public void RejectsNonAllowlistedRootType() + { + // System.IO.FileInfo is a classic gadget-chain probe. The binder must reject it + // even though the BCL would happily resolve it via Type.GetType. + string json = "{\"$type\":\"System.IO.FileInfo, System.Private.CoreLib\",\"OriginalPath\":\"c:\\\\evil\"}"; + Assert.ThrowsException( + () => JsonConvert.DeserializeObject(json, ReadJsonSettings)); + } + + [TestMethod] + public void RejectsNonAllowlistedNestedType() + { + // Embed a malicious $type inside an otherwise valid ExecutionStartedEvent's Tags + // member. The Tags member is IDictionary, so the $type token is + // honored by Newtonsoft.Json when reading. Anything other than + // Dictionary must be rejected. + string json = "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," + + "\"Tags\":{\"$type\":\"System.Collections.Generic.SortedDictionary`2[[System.String, System.Private.CoreLib],[System.String, System.Private.CoreLib]], System.Collections\"}}"; + Assert.ThrowsException( + () => JsonConvert.DeserializeObject(json, ReadJsonSettings)); + } + + [TestMethod] + public void AllowsDictionaryStringStringForTags() + { + // The exact concrete type Newtonsoft.Json emits for IDictionary Tags. + string json = "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," + + "\"Tags\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, " + typeof(string).Assembly.GetName().Name + "],[System.String, " + typeof(string).Assembly.GetName().Name + "]], " + typeof(Dictionary).Assembly.GetName().Name + "\"," + + "\"tag1\":\"v1\"}}"; + + var deserialized = (ExecutionStartedEvent)JsonConvert.DeserializeObject(json, ReadJsonSettings); + Assert.AreEqual("v1", deserialized.Tags["tag1"]); + } + } +} diff --git a/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs b/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs index 99272b67d..a292fd144 100644 --- a/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs +++ b/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs @@ -18,7 +18,6 @@ namespace DurableTask.ServiceBus.Tracking using System.Runtime.Serialization; using Azure.Data.Tables; using DurableTask.Core.History; - using DurableTask.Core.Serializing; using Newtonsoft.Json; /// @@ -35,7 +34,7 @@ internal class AzureTableOrchestrationHistoryEventEntity : AzureTableCompositeTa private static readonly JsonSerializerSettings ReadJsonSettings = new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Objects, - SerializationBinder = new PackageUpgradeSerializationBinder() + SerializationBinder = new HistoryEventSerializationBinder() }; /// diff --git a/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs b/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs new file mode 100644 index 000000000..c48932367 --- /dev/null +++ b/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs @@ -0,0 +1,67 @@ +// ---------------------------------------------------------------------------------- +// 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.ServiceBus.Tracking +{ + using System; + using System.Collections.Generic; + using System.Reflection; + using DurableTask.Core.History; + using DurableTask.Core.Serializing; + using Newtonsoft.Json; + + /// + /// Strict used when deserializing + /// orchestration history events stored in Azure Table tracking. Only types that the framework + /// itself emits when serializing a are accepted; any other + /// $type token is rejected with a . + /// + /// + /// This binder is intentionally restrictive: the history event JSON written by DTFx never + /// contains polymorphic customer types (customer payloads such as Input/Result/ + /// Reason/Details are pre-serialized strings on the + /// subtree and are opaque to this serializer). Restricting the resolvable type set defends + /// against unsafe-deserialization gadget chains if a malicious $type were ever written + /// into the tracking table by an attacker with write access to the Storage account. + /// + internal sealed class HistoryEventSerializationBinder : PackageUpgradeSerializationBinder + { + static readonly Assembly DurableTaskCoreAssembly = typeof(HistoryEvent).Assembly; + + /// + public override Type BindToType(string assemblyName, string typeName) + { + Type resolved = base.BindToType(assemblyName, typeName); + + if (resolved == null || !IsAllowed(resolved)) + { + throw new JsonSerializationException( + $"Type '{typeName}' from assembly '{assemblyName}' is not permitted by the orchestration history serialization binder."); + } + + return resolved; + } + + static bool IsAllowed(Type type) + { + // Allow any type defined in DurableTask.Core (HistoryEvent subclasses, OrchestrationInstance, + // ParentInstance, FailureDetails, OrchestrationExecutionContext, etc.), plus + // Dictionary to round-trip the IDictionary Tags members + // declared on ExecutionStartedEvent / SubOrchestrationInstanceCreatedEvent / + // TaskScheduledEvent (Newtonsoft.Json emits a $type for these because the static + // declared type is an interface). + return type.Assembly == DurableTaskCoreAssembly + || type == typeof(Dictionary); + } + } +} From a10762927bd1a45a5cb6667f20c29277e6bf199e Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Fri, 1 May 2026 13:28:10 -0700 Subject: [PATCH 2/5] Enhance HistoryEventSerializationBinder with assembly allowlist checks for improved security --- .../HistoryEventSerializationBinderTests.cs | 46 ++++++++++++++++--- .../HistoryEventSerializationBinder.cs | 37 +++++++++++++++ 2 files changed, 76 insertions(+), 7 deletions(-) diff --git a/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs b/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs index 7acb84313..7a25c468c 100644 --- a/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs +++ b/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs @@ -15,6 +15,7 @@ namespace DurableTask.ServiceBus.Tests { using System; using System.Collections.Generic; + using System.IO; using DurableTask.Core; using DurableTask.Core.History; using DurableTask.ServiceBus.Tracking; @@ -79,8 +80,15 @@ public void AllowsAllConcreteHistoryEventTypes() public void RejectsNonAllowlistedRootType() { // System.IO.FileInfo is a classic gadget-chain probe. The binder must reject it - // even though the BCL would happily resolve it via Type.GetType. - string json = "{\"$type\":\"System.IO.FileInfo, System.Private.CoreLib\",\"OriginalPath\":\"c:\\\\evil\"}"; + // even though the BCL would happily resolve it via Type.GetType. FileInfo lives + // in the BCL (which is on the assembly-name allowlist so that Dictionary can round-trip), so this test specifically exercises the post-resolution + // IsAllowed filter rather than the assembly-name pre-filter. + // The BCL assembly name differs between TFMs (System.Private.CoreLib on .NET, + // mscorlib on .NET Framework), so build the $type string from the actual runtime + // type to avoid passing for the wrong reason (type-resolution failure). + string bclAssemblyName = typeof(FileInfo).Assembly.GetName().Name; + string json = $"{{\"$type\":\"System.IO.FileInfo, {bclAssemblyName}\",\"OriginalPath\":\"c:\\\\evil\"}}"; Assert.ThrowsException( () => JsonConvert.DeserializeObject(json, ReadJsonSettings)); } @@ -91,9 +99,15 @@ public void RejectsNonAllowlistedNestedType() // Embed a malicious $type inside an otherwise valid ExecutionStartedEvent's Tags // member. The Tags member is IDictionary, so the $type token is // honored by Newtonsoft.Json when reading. Anything other than - // Dictionary must be rejected. - string json = "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," - + "\"Tags\":{\"$type\":\"System.Collections.Generic.SortedDictionary`2[[System.String, System.Private.CoreLib],[System.String, System.Private.CoreLib]], System.Collections\"}}"; + // Dictionary must be rejected. SortedDictionary lives in a non-BCL + // assembly (System.Collections on .NET, System on .NET Framework) so this test + // exercises the assembly-name pre-filter. + // Build $type strings dynamically so they resolve under both target frameworks. + string bclAssemblyName = typeof(string).Assembly.GetName().Name; + string sortedDictAssemblyName = typeof(SortedDictionary).Assembly.GetName().Name; + string json = + "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," + + $"\"Tags\":{{\"$type\":\"System.Collections.Generic.SortedDictionary`2[[System.String, {bclAssemblyName}],[System.String, {bclAssemblyName}]], {sortedDictAssemblyName}\"}}}}"; Assert.ThrowsException( () => JsonConvert.DeserializeObject(json, ReadJsonSettings)); } @@ -102,12 +116,30 @@ public void RejectsNonAllowlistedNestedType() public void AllowsDictionaryStringStringForTags() { // The exact concrete type Newtonsoft.Json emits for IDictionary Tags. - string json = "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," - + "\"Tags\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, " + typeof(string).Assembly.GetName().Name + "],[System.String, " + typeof(string).Assembly.GetName().Name + "]], " + typeof(Dictionary).Assembly.GetName().Name + "\"," + string bclAssemblyName = typeof(string).Assembly.GetName().Name; + string dictAssemblyName = typeof(Dictionary).Assembly.GetName().Name; + string json = + "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," + + $"\"Tags\":{{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, {bclAssemblyName}],[System.String, {bclAssemblyName}]], {dictAssemblyName}\"," + "\"tag1\":\"v1\"}}"; var deserialized = (ExecutionStartedEvent)JsonConvert.DeserializeObject(json, ReadJsonSettings); Assert.AreEqual("v1", deserialized.Tags["tag1"]); } + + [TestMethod] + public void AllowsLegacyDurableTaskAssemblyNameRewrite() + { + // Pre-v2 DTFx payloads were written with assembly name 'DurableTask' (or + // 'DurableTaskFx') and a 'DurableTask.' type name. PackageUpgradeSerializationBinder + // (the base class) rewrites these to 'DurableTask.Core.' for upgrade compatibility. + // This test guards that path: HistoryEventSerializationBinder must keep the legacy + // assembly name on its allowlist and must accept the rewritten type after resolution. + var binder = new HistoryEventSerializationBinder(); + Type bound = binder.BindToType( + assemblyName: "DurableTask", + typeName: "DurableTask.History.ExecutionStartedEvent"); + Assert.AreEqual(typeof(ExecutionStartedEvent), bound); + } } } diff --git a/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs b/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs index c48932367..c3dbc5e8c 100644 --- a/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs +++ b/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs @@ -38,11 +38,38 @@ internal sealed class HistoryEventSerializationBinder : PackageUpgradeSerializat { static readonly Assembly DurableTaskCoreAssembly = typeof(HistoryEvent).Assembly; + // Allowlist of simple assembly names whose types may even be resolved by this binder. + // This is a defense-in-depth check applied *before* delegating to the base binder, so + // that the .NET runtime never has to load (and execute the module initializer of) an + // assembly that is not on the allowlist as a side effect of probing a malicious $type. + static readonly HashSet AllowedAssemblySimpleNames = new HashSet(StringComparer.Ordinal) + { + DurableTaskCoreAssembly.GetName().Name, // DurableTask.Core + typeof(Dictionary).Assembly.GetName().Name, // BCL host of Dictionary; differs across TFMs + "DurableTask", // pre-v2 DTFx assembly (legacy upgrade path) + "DurableTaskFx", // pre-v2 DTFx vNext assembly (legacy upgrade path) + }; + /// public override Type BindToType(string assemblyName, string typeName) { + // Stage 1: reject by assembly name string before invoking the base binder so that + // unknown assemblies are never loaded just to be rejected afterwards. + string simpleAssemblyName = ExtractSimpleAssemblyName(assemblyName); + if (simpleAssemblyName != null && !AllowedAssemblySimpleNames.Contains(simpleAssemblyName)) + { + throw new JsonSerializationException( + $"Type '{typeName}' from assembly '{assemblyName}' is not permitted by the orchestration history serialization binder."); + } + + // Stage 2: delegate to PackageUpgradeSerializationBinder for the legacy + // DurableTask.* -> DurableTask.Core.* rewrite and standard type resolution. Type resolved = base.BindToType(assemblyName, typeName); + // Stage 3: filter the resolved type. The BCL is in the assembly allowlist (so that + // Dictionary can be round-tripped for the Tags member), so we still + // need this post-resolution check to reject other BCL types (e.g., FileInfo, + // ObjectDataProvider) that an attacker might try to use as a deserialization gadget. if (resolved == null || !IsAllowed(resolved)) { throw new JsonSerializationException( @@ -63,5 +90,15 @@ static bool IsAllowed(Type type) return type.Assembly == DurableTaskCoreAssembly || type == typeof(Dictionary); } + + static string ExtractSimpleAssemblyName(string assemblyName) + { + if (string.IsNullOrWhiteSpace(assemblyName)) + { + return null; + } + int comma = assemblyName.IndexOf(','); + return comma < 0 ? assemblyName : assemblyName.Substring(0, comma); + } } } From 0d0f13a863ea4d971ecc6e01b8ba9ecdab06722f Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Fri, 1 May 2026 13:46:39 -0700 Subject: [PATCH 3/5] Suppress the original CodeQL alerts as addressed --- .../Tracking/AzureTableOrchestrationHistoryEventEntity.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs b/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs index a292fd144..219474000 100644 --- a/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs +++ b/src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs @@ -28,11 +28,13 @@ internal class AzureTableOrchestrationHistoryEventEntity : AzureTableCompositeTa private static readonly JsonSerializerSettings WriteJsonSettings = new JsonSerializerSettings { Formatting = Formatting.Indented, + // CodeQL [SM02211] Serialization-only path; deserialization is constrained by HistoryEventSerializationBinder on ReadJsonSettings. TypeNameHandling = TypeNameHandling.Objects }; private static readonly JsonSerializerSettings ReadJsonSettings = new JsonSerializerSettings { + // CodeQL [SM02211] Constrained by HistoryEventSerializationBinder allowlist; only DurableTask.Core types and Dictionary are accepted. TypeNameHandling = TypeNameHandling.Objects, SerializationBinder = new HistoryEventSerializationBinder() }; From 05f7cbd5acbcfa1e62b8c7f1d9ce9fc2eb69b8bb Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Fri, 1 May 2026 14:28:40 -0700 Subject: [PATCH 4/5] Address Copilot PR review: harden binder and tests - Reject null/empty assembly names in BindToType to avoid NRE from base PackageUpgradeSerializationBinder when $type lacks an assembly portion. - Broaden assembly allowlist to host assemblies of common IDictionary implementations (Dictionary, SortedDictionary, ConcurrentDictionary, ReadOnlyDictionary) so historical Tags persisted with non-Dictionary runtime types continue to deserialize after public APIs accept any IDictionary. - Replace exact-Dictionary post-check with IDictionary-shape check; gadget types in the same BCL assembly (e.g., FileInfo) are still rejected. - Add tests: RejectsNullAssemblyName, RejectsNonBclDictionaryAssembly, AllowsSortedDictionaryStringStringForTags. Update RejectsNonAllowlistedNestedType to use Hashtable (BCL but non-IDictionary) to exercise the post-resolution shape filter. --- .../HistoryEventSerializationBinderTests.cs | 58 ++++++++++++++++--- .../HistoryEventSerializationBinder.cs | 46 +++++++++++---- 2 files changed, 84 insertions(+), 20 deletions(-) diff --git a/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs b/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs index 7a25c468c..405862f64 100644 --- a/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs +++ b/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs @@ -98,20 +98,44 @@ public void RejectsNonAllowlistedNestedType() { // Embed a malicious $type inside an otherwise valid ExecutionStartedEvent's Tags // member. The Tags member is IDictionary, so the $type token is - // honored by Newtonsoft.Json when reading. Anything other than - // Dictionary must be rejected. SortedDictionary lives in a non-BCL - // assembly (System.Collections on .NET, System on .NET Framework) so this test - // exercises the assembly-name pre-filter. - // Build $type strings dynamically so they resolve under both target frameworks. - string bclAssemblyName = typeof(string).Assembly.GetName().Name; - string sortedDictAssemblyName = typeof(SortedDictionary).Assembly.GetName().Name; + // honored by Newtonsoft.Json when reading. The binder must reject any concrete + // type that is not assignable to IDictionary even when it lives in + // an allowlisted assembly. System.Collections.Hashtable is in the BCL (so the + // assembly-name pre-filter does not apply) but is not IDictionary, + // so this exercises the post-resolution IsAllowed filter. + string bclAssemblyName = typeof(System.Collections.Hashtable).Assembly.GetName().Name; + string json = + "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," + + $"\"Tags\":{{\"$type\":\"System.Collections.Hashtable, {bclAssemblyName}\"}}}}"; + Assert.ThrowsException( + () => JsonConvert.DeserializeObject(json, ReadJsonSettings)); + } + + [TestMethod] + public void RejectsNonBclDictionaryAssembly() + { + // Even an IDictionary implementation must originate from an + // allowlisted assembly. Construct a $type referencing a fictitious assembly to + // confirm the assembly-name pre-filter rejects it before any type loading occurs. string json = "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," - + $"\"Tags\":{{\"$type\":\"System.Collections.Generic.SortedDictionary`2[[System.String, {bclAssemblyName}],[System.String, {bclAssemblyName}]], {sortedDictAssemblyName}\"}}}}"; + + "\"Tags\":{\"$type\":\"Some.Evil.Dictionary, Some.Evil.Assembly\"}}"; Assert.ThrowsException( () => JsonConvert.DeserializeObject(json, ReadJsonSettings)); } + [TestMethod] + public void RejectsNullAssemblyName() + { + // Json.NET can invoke BindToType with a null assemblyName when an incoming $type + // token omits the assembly portion. The binder must fail deterministically with a + // JsonSerializationException rather than letting a NullReferenceException leak out + // of PackageUpgradeSerializationBinder. + var binder = new HistoryEventSerializationBinder(); + Assert.ThrowsException( + () => binder.BindToType(assemblyName: null, typeName: "DurableTask.Core.History.ExecutionStartedEvent")); + } + [TestMethod] public void AllowsDictionaryStringStringForTags() { @@ -127,6 +151,24 @@ public void AllowsDictionaryStringStringForTags() Assert.AreEqual("v1", deserialized.Tags["tag1"]); } + [TestMethod] + public void AllowsSortedDictionaryStringStringForTags() + { + // Public DTFx APIs accept any IDictionary for Tags and persist it + // with its runtime $type. SortedDictionary lives in a different BCL assembly than + // Dictionary on .NET (System.Collections), so this test exercises the multi-host + // assembly allowlist together with the IDictionary-shape post-check. + string bclAssemblyName = typeof(string).Assembly.GetName().Name; + string sortedDictAssemblyName = typeof(SortedDictionary).Assembly.GetName().Name; + string json = + "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\"," + + $"\"Tags\":{{\"$type\":\"System.Collections.Generic.SortedDictionary`2[[System.String, {bclAssemblyName}],[System.String, {bclAssemblyName}]], {sortedDictAssemblyName}\"," + + "\"tag1\":\"v1\"}}"; + + var deserialized = (ExecutionStartedEvent)JsonConvert.DeserializeObject(json, ReadJsonSettings); + Assert.AreEqual("v1", deserialized.Tags["tag1"]); + } + [TestMethod] public void AllowsLegacyDurableTaskAssemblyNameRewrite() { diff --git a/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs b/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs index c3dbc5e8c..cf001175e 100644 --- a/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs +++ b/src/DurableTask.ServiceBus/Tracking/HistoryEventSerializationBinder.cs @@ -42,12 +42,22 @@ internal sealed class HistoryEventSerializationBinder : PackageUpgradeSerializat // This is a defense-in-depth check applied *before* delegating to the base binder, so // that the .NET runtime never has to load (and execute the module initializer of) an // assembly that is not on the allowlist as a side effect of probing a malicious $type. + // The set spans: + // * DurableTask.Core (the source of HistoryEvent and friends). + // * The legacy pre-v2 DTFx assembly names rewritten by PackageUpgradeSerializationBinder. + // * The BCL assemblies that host common IDictionary implementations + // emitted as $type for the Tags member. The host assembly varies by TFM and by + // concrete dictionary type, so we enumerate them via typeof(...).Assembly to stay + // correct under multi-targeting. static readonly HashSet AllowedAssemblySimpleNames = new HashSet(StringComparer.Ordinal) { - DurableTaskCoreAssembly.GetName().Name, // DurableTask.Core - typeof(Dictionary).Assembly.GetName().Name, // BCL host of Dictionary; differs across TFMs - "DurableTask", // pre-v2 DTFx assembly (legacy upgrade path) - "DurableTaskFx", // pre-v2 DTFx vNext assembly (legacy upgrade path) + DurableTaskCoreAssembly.GetName().Name, // DurableTask.Core + typeof(Dictionary).Assembly.GetName().Name, // mscorlib / System.Private.CoreLib + typeof(SortedDictionary).Assembly.GetName().Name, // System / System.Collections + typeof(System.Collections.Concurrent.ConcurrentDictionary).Assembly.GetName().Name, // mscorlib / System.Collections.Concurrent + typeof(System.Collections.ObjectModel.ReadOnlyDictionary).Assembly.GetName().Name, // mscorlib / System.ObjectModel + "DurableTask", // pre-v2 DTFx assembly (legacy upgrade path) + "DurableTaskFx", // pre-v2 DTFx vNext assembly (legacy upgrade path) }; /// @@ -55,8 +65,13 @@ public override Type BindToType(string assemblyName, string typeName) { // Stage 1: reject by assembly name string before invoking the base binder so that // unknown assemblies are never loaded just to be rejected afterwards. + // Reject null/empty assembly names deterministically. Json.NET can invoke + // BindToType with a null assemblyName when an incoming $type token omits the + // assembly portion; an unqualified type name is never produced by DTFx + // serialization and would also cause the base PackageUpgradeSerializationBinder + // to throw a NullReferenceException, so fail fast with a typed exception here. string simpleAssemblyName = ExtractSimpleAssemblyName(assemblyName); - if (simpleAssemblyName != null && !AllowedAssemblySimpleNames.Contains(simpleAssemblyName)) + if (simpleAssemblyName == null || !AllowedAssemblySimpleNames.Contains(simpleAssemblyName)) { throw new JsonSerializationException( $"Type '{typeName}' from assembly '{assemblyName}' is not permitted by the orchestration history serialization binder."); @@ -82,13 +97,20 @@ public override Type BindToType(string assemblyName, string typeName) static bool IsAllowed(Type type) { // Allow any type defined in DurableTask.Core (HistoryEvent subclasses, OrchestrationInstance, - // ParentInstance, FailureDetails, OrchestrationExecutionContext, etc.), plus - // Dictionary to round-trip the IDictionary Tags members - // declared on ExecutionStartedEvent / SubOrchestrationInstanceCreatedEvent / - // TaskScheduledEvent (Newtonsoft.Json emits a $type for these because the static - // declared type is an interface). - return type.Assembly == DurableTaskCoreAssembly - || type == typeof(Dictionary); + // ParentInstance, FailureDetails, OrchestrationExecutionContext, etc.). + if (type.Assembly == DurableTaskCoreAssembly) + { + return true; + } + + // For all other allowlisted assemblies (the BCL dictionary hosts plus the legacy + // pre-v2 DTFx assemblies), only types assignable to IDictionary + // are accepted. This narrows the resolved set so that gadget types living in the + // same allowlisted BCL assembly (e.g., FileInfo in System.Private.CoreLib) cannot + // pass the post-resolution check. Tags is the only IDictionary + // member declared on a serialized HistoryEvent subtree, so any other concrete + // type is unreachable by the legitimate serializer in any case. + return typeof(IDictionary).IsAssignableFrom(type); } static string ExtractSimpleAssemblyName(string assemblyName) From fe787bd76d7ab24fb45e2fadc044b4e1702c4252 Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Fri, 1 May 2026 14:28:56 -0700 Subject: [PATCH 5/5] Move HistoryEventSerializationBinderTests.cs into lowercase test/ folder The ServiceBus test csproj lives at test/DurableTask.ServiceBus.Tests, but the file was committed under Test/ (capital T). On case-sensitive filesystems (Linux CI) the SDK glob rooted at the lowercase csproj folder would not pick this file up, meaning the new binder tests would not run in CI. --- .../HistoryEventSerializationBinderTests.cs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename {Test => test}/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs (100%) diff --git a/Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs b/test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs similarity index 100% rename from Test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs rename to test/DurableTask.ServiceBus.Tests/HistoryEventSerializationBinderTests.cs