From 2da05b0d73252725335b8da2d6aa4fbe80d48e1c Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Thu, 30 Apr 2026 18:50:04 -0700 Subject: [PATCH 1/2] Add TraceContextSerializationBinder to constrain deserialization Co-authored-by: Copilot --- .../TraceContextSerializationBinderTests.cs | 60 +++++++++++++++++++ .../TraceContextSerializationBinder.cs | 60 +++++++++++++++++++ src/DurableTask.Core/TraceContextBase.cs | 2 + 3 files changed, 122 insertions(+) create mode 100644 Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs create mode 100644 src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs diff --git a/Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs b/Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs new file mode 100644 index 000000000..758b9cd2d --- /dev/null +++ b/Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs @@ -0,0 +1,60 @@ +// ---------------------------------------------------------------------------------- +// 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.Core.Tests +{ + using System; + using Microsoft.VisualStudio.TestTools.UnitTesting; + + [TestClass] + public class TraceContextSerializationBinderTests + { + [TestMethod] + public void RejectsNonTraceContextRootType() + { + // Root $type resolves to System.String, which is not a TraceContextBase subclass. + // Restore's pre-binder check rejects with a generic Exception. + string json = "{\"$type\":\"System.String\",\"Value\":\"evil\"}"; + Assert.ThrowsException(() => TraceContextBase.Restore(json)); + } + + [TestMethod] + public void RejectsNonAllowlistedNestedType() + { + // Root is a legitimate W3CTraceContext, but a nested $type points outside the allowlist. + string json = "{\"$type\":\"DurableTask.Core.W3CTraceContext, DurableTask.Core\"," + + "\"OrchestrationTraceContexts\":[{\"$type\":\"System.String\",\"Value\":\"evil\"}]}"; + Assert.ThrowsException( + () => TraceContextBase.Restore(json)); + } + + [TestMethod] + public void AllowsLegitimateW3CTraceContextRoundTrip() + { + string json = "{\"$id\":\"1\",\"$type\":\"DurableTask.Core.W3CTraceContext, DurableTask.Core\"," + + "\"TraceParent\":\"00-a422532de19d3e4f8f67af06f8f880c7-81354b086ec6fb41-02\"," + + "\"TraceState\":null,\"ParentSpanId\":\"b69bc0f95af84240\"," + + "\"StartTime\":\"2019-05-03T23:43:27.6728211+00:00\"," + + "\"OrchestrationTraceContexts\":[{\"$id\":\"2\",\"$type\":\"DurableTask.Core.W3CTraceContext, DurableTask.Core\"," + + "\"TraceParent\":\"00-a422532de19d3e4f8f67af06f8f880c7-f86a8711d7226d42-02\"," + + "\"TraceState\":null,\"ParentSpanId\":\"2ec2a64f22dbb143\"," + + "\"StartTime\":\"2019-05-03T23:43:12.7553182+00:00\"," + + "\"OrchestrationTraceContexts\":[{\"$ref\":\"2\"}]}]}"; + + TraceContextBase context = TraceContextBase.Restore(json); + + Assert.IsInstanceOfType(context, typeof(W3CTraceContext)); + Assert.AreEqual(DateTimeOffset.Parse("2019-05-03T23:43:27.6728211+00:00"), context.StartTime); + } + } +} diff --git a/src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs b/src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs new file mode 100644 index 000000000..00204386a --- /dev/null +++ b/src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs @@ -0,0 +1,60 @@ +// ---------------------------------------------------------------------------------- +// 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.Core.Serializing +{ + using System; + using System.Collections.Generic; + using Newtonsoft.Json; + + /// + /// Strict used by + /// when (de)serializing trace-context payloads that flow with + /// orchestration messages. Only and its subclasses are accepted; + /// any other $type token is rejected with a . + /// + internal sealed class TraceContextSerializationBinder : PackageUpgradeSerializationBinder + { + static readonly Type TraceContextBaseType = typeof(TraceContextBase); + static readonly Type StackOfTraceContextBaseType = typeof(Stack); + + /// + public override Type BindToType(string assemblyName, string typeName) + { + Type resolved = base.BindToType(assemblyName, typeName); + + if (!IsAllowed(resolved)) + { + throw new JsonSerializationException( + $"Type '{resolved.FullName}, {resolved.Assembly.GetName().Name}' is not permitted by the trace-context type allowlist."); + } + + return resolved; + } + + static bool IsAllowed(Type type) + { + if (type == TraceContextBaseType || type.IsSubclassOf(TraceContextBaseType)) + { + return true; + } + + if (type == StackOfTraceContextBaseType) + { + return true; + } + + return false; + } + } +} diff --git a/src/DurableTask.Core/TraceContextBase.cs b/src/DurableTask.Core/TraceContextBase.cs index fceadb768..4764d1511 100644 --- a/src/DurableTask.Core/TraceContextBase.cs +++ b/src/DurableTask.Core/TraceContextBase.cs @@ -19,6 +19,7 @@ namespace DurableTask.Core using System.Linq; using System.Reflection; using DurableTask.Core.Common; + using DurableTask.Core.Serializing; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -44,6 +45,7 @@ static TraceContextBase() TypeNameHandling = TypeNameHandling.Objects, PreserveReferencesHandling = PreserveReferencesHandling.Objects, ReferenceLoopHandling = ReferenceLoopHandling.Serialize, + SerializationBinder = new TraceContextSerializationBinder(), }; serializer = JsonSerializer.Create(CustomJsonSerializerSettings); From d330f2cf317357dd31469b9312e383e43a6fa73c Mon Sep 17 00:00:00 2001 From: Anatoli Beliaev Date: Fri, 1 May 2026 16:17:12 -0700 Subject: [PATCH 2/2] Enhance TraceContextSerializationBinder to improve type safety and restrict deserialization --- .../TraceContextSerializationBinderTests.cs | 115 +++++++++++++++++- .../TraceContextSerializationBinder.cs | 69 ++++++++++- 2 files changed, 178 insertions(+), 6 deletions(-) diff --git a/Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs b/Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs index 758b9cd2d..daa83d682 100644 --- a/Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs +++ b/Test/DurableTask.Core.Tests/TraceContextSerializationBinderTests.cs @@ -14,7 +14,11 @@ namespace DurableTask.Core.Tests { using System; + using System.Collections.Generic; + using System.IO; + using DurableTask.Core.Serializing; using Microsoft.VisualStudio.TestTools.UnitTesting; + using Newtonsoft.Json; [TestClass] public class TraceContextSerializationBinderTests @@ -34,7 +38,7 @@ public void RejectsNonAllowlistedNestedType() // Root is a legitimate W3CTraceContext, but a nested $type points outside the allowlist. string json = "{\"$type\":\"DurableTask.Core.W3CTraceContext, DurableTask.Core\"," + "\"OrchestrationTraceContexts\":[{\"$type\":\"System.String\",\"Value\":\"evil\"}]}"; - Assert.ThrowsException( + Assert.ThrowsException( () => TraceContextBase.Restore(json)); } @@ -56,5 +60,114 @@ public void AllowsLegitimateW3CTraceContextRoundTrip() Assert.IsInstanceOfType(context, typeof(W3CTraceContext)); Assert.AreEqual(DateTimeOffset.Parse("2019-05-03T23:43:27.6728211+00:00"), context.StartTime); } + + [TestMethod] + public void AllowsActualSerializeDeserializeRoundTrip() + { + // End-to-end check: SerializableTraceContext produces a payload that Restore + // accepts. This guards against a binder allowlist that is too tight to round-trip + // a legitimately serialized TraceContext. + var original = new W3CTraceContext + { + TraceParent = "00-a422532de19d3e4f8f67af06f8f880c7-81354b086ec6fb41-02", + ParentSpanId = "b69bc0f95af84240", + StartTime = DateTimeOffset.Parse("2019-05-03T23:43:27.6728211+00:00"), + OperationName = "TestOp", + TelemetryType = TelemetryType.Request, + }; + + string json = original.SerializableTraceContext; + TraceContextBase restored = TraceContextBase.Restore(json); + + Assert.IsInstanceOfType(restored, typeof(W3CTraceContext)); + Assert.AreEqual(original.StartTime, restored.StartTime); + Assert.AreEqual(original.OperationName, restored.OperationName); + } + + [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 TraceContextSerializationBinder(); + Assert.ThrowsException( + () => binder.BindToType(assemblyName: null, typeName: "DurableTask.Core.W3CTraceContext")); + } + + [TestMethod] + public void RejectsNonAllowlistedAssembly() + { + // Even a type *named* like a TraceContext must be rejected if it claims to live in + // a non-allowlisted assembly. The pre-filter rejects without loading the assembly. + var binder = new TraceContextSerializationBinder(); + Assert.ThrowsException( + () => binder.BindToType( + assemblyName: "Some.Evil.Assembly", + typeName: "DurableTask.Core.W3CTraceContext")); + } + + [TestMethod] + public void RejectsBclGadgetTypeInAllowlistedAssembly() + { + // System.IO.FileInfo is a classic gadget-chain probe. It lives in the BCL (which + // is on the assembly-name allowlist so that Stack can be bound), + // 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 + // assembly name from the runtime type to avoid hard-coding it. + string bclAssemblyName = typeof(FileInfo).Assembly.GetName().Name; + var binder = new TraceContextSerializationBinder(); + Assert.ThrowsException( + () => binder.BindToType(assemblyName: bclAssemblyName, typeName: "System.IO.FileInfo")); + } + + [TestMethod] + public void AllowsAllConcreteTraceContextTypes() + { + // Sanity check that every concrete TraceContextBase subclass declared in + // DurableTask.Core is accepted by the binder. + var binder = new TraceContextSerializationBinder(); + string assemblyName = typeof(TraceContextBase).Assembly.GetName().Name; + foreach (Type concreteType in new[] + { + typeof(W3CTraceContext), + typeof(HttpCorrelationProtocolTraceContext), + typeof(NullObjectTraceContext), + }) + { + Type bound = binder.BindToType(assemblyName, concreteType.FullName); + Assert.AreEqual(concreteType, bound); + } + } + + [TestMethod] + public void AllowsStackOfTraceContextBase() + { + // The Stack generic closed type is on the allowlist so that the + // OrchestrationTraceContexts member can be bound. Confirm that direct lookup works. + var binder = new TraceContextSerializationBinder(); + Type stackType = typeof(Stack); + string typeName = stackType.FullName; + string assemblyName = stackType.Assembly.GetName().Name; + Type bound = binder.BindToType(assemblyName, typeName); + Assert.AreEqual(stackType, bound); + } + + [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: TraceContextSerializationBinder must keep the legacy + // assembly name on its allowlist and must accept the rewritten type after resolution. + var binder = new TraceContextSerializationBinder(); + Type bound = binder.BindToType( + assemblyName: "DurableTask", + typeName: "DurableTask.W3CTraceContext"); + Assert.AreEqual(typeof(W3CTraceContext), bound); + } } } diff --git a/src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs b/src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs index 00204386a..ec3ec84c4 100644 --- a/src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs +++ b/src/DurableTask.Core/Serializing/TraceContextSerializationBinder.cs @@ -15,28 +15,71 @@ namespace DurableTask.Core.Serializing { using System; using System.Collections.Generic; + using System.Reflection; using Newtonsoft.Json; /// /// Strict used by /// when (de)serializing trace-context payloads that flow with - /// orchestration messages. Only and its subclasses are accepted; - /// any other $type token is rejected with a . + /// orchestration messages. Only and its subclasses (plus + /// for the OrchestrationTraceContexts member) + /// are accepted; any other $type token is rejected with a + /// . /// + /// + /// Restricting the resolvable type set defends against unsafe-deserialization gadget chains + /// if a malicious $type were ever injected into a trace-context payload by an attacker + /// with the ability to write into the orchestration message stream. + /// internal sealed class TraceContextSerializationBinder : PackageUpgradeSerializationBinder { static readonly Type TraceContextBaseType = typeof(TraceContextBase); static readonly Type StackOfTraceContextBaseType = typeof(Stack); + static readonly Assembly DurableTaskCoreAssembly = typeof(TraceContextBase).Assembly; + + // Allowlist of simple assembly names whose types may even be resolved by this binder. + // This is a defense-in-depth pre-filter 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 TraceContextBase and friends). + // * The BCL host of Stack for the OrchestrationTraceContexts member. + // * The legacy pre-v2 DTFx assembly names rewritten by PackageUpgradeSerializationBinder. + static readonly HashSet AllowedAssemblySimpleNames = new HashSet(StringComparer.Ordinal) + { + DurableTaskCoreAssembly.GetName().Name, // DurableTask.Core + StackOfTraceContextBaseType.Assembly.GetName().Name, // System.Collections / mscorlib / System.Private.CoreLib + "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. Also rejects + // null/empty assembly names deterministically: Json.NET can invoke BindToType with a + // null assemblyName when an incoming $type token omits the assembly portion, and the + // base PackageUpgradeSerializationBinder would NRE in that case. + string simpleAssemblyName = ExtractSimpleAssemblyName(assemblyName); + if (simpleAssemblyName == null || !AllowedAssemblySimpleNames.Contains(simpleAssemblyName)) + { + throw new JsonSerializationException( + $"Type '{typeName}' from assembly '{assemblyName}' is not permitted by the trace-context serialization binder."); + } + + // Stage 2: delegate to PackageUpgradeSerializationBinder for the legacy + // DurableTask.* -> DurableTask.Core.* rewrite and standard type resolution. Type resolved = base.BindToType(assemblyName, typeName); - if (!IsAllowed(resolved)) + // Stage 3: filter the resolved type. Even though Stage 1 narrowed the assembly set, + // the BCL is in scope (so that Stack can round-trip), so we still + // need this post-resolution check to reject other BCL types that an attacker might + // try to use as a deserialization gadget. + if (resolved == null || !IsAllowed(resolved)) { throw new JsonSerializationException( - $"Type '{resolved.FullName}, {resolved.Assembly.GetName().Name}' is not permitted by the trace-context type allowlist."); + $"Type '{typeName}' from assembly '{assemblyName}' is not permitted by the trace-context serialization binder."); } return resolved; @@ -44,11 +87,17 @@ public override Type BindToType(string assemblyName, string typeName) static bool IsAllowed(Type type) { - if (type == TraceContextBaseType || type.IsSubclassOf(TraceContextBaseType)) + // TraceContextBase itself or any subclass (W3CTraceContext, HttpCorrelationProtocolTraceContext, + // NullObjectTraceContext, plus any future subclass added in DurableTask.Core). + if (TraceContextBaseType.IsAssignableFrom(type)) { return true; } + // Stack may be requested when the static type of the + // OrchestrationTraceContexts property is bound. The generic argument's identity is + // already constrained by the type system, so allowing this exact closed generic + // type does not widen the gadget surface beyond TraceContextBase itself. if (type == StackOfTraceContextBaseType) { return true; @@ -56,5 +105,15 @@ static bool IsAllowed(Type type) return false; } + + static string ExtractSimpleAssemblyName(string assemblyName) + { + if (string.IsNullOrWhiteSpace(assemblyName)) + { + return null; + } + int comma = assemblyName.IndexOf(','); + return comma < 0 ? assemblyName : assemblyName.Substring(0, comma); + } } }