Skip to content

Commit b168f20

Browse files
committed
Add HistoryEventSerializationBinder to constrain HistoryEvent deserialization
Mitigates https://msazure.visualstudio.com/Antares/_workitems/edit/37181649
1 parent 553d281 commit b168f20

3 files changed

Lines changed: 181 additions & 2 deletions

File tree

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
// ----------------------------------------------------------------------------------
2+
// Copyright Microsoft Corporation
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
// Unless required by applicable law or agreed to in writing, software
8+
// distributed under the License is distributed on an "AS IS" BASIS,
9+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
// See the License for the specific language governing permissions and
11+
// limitations under the License.
12+
// ----------------------------------------------------------------------------------
13+
14+
namespace DurableTask.ServiceBus.Tests
15+
{
16+
using System;
17+
using System.Collections.Generic;
18+
using DurableTask.Core;
19+
using DurableTask.Core.History;
20+
using DurableTask.ServiceBus.Tracking;
21+
using Microsoft.VisualStudio.TestTools.UnitTesting;
22+
using Newtonsoft.Json;
23+
24+
[TestClass]
25+
public class HistoryEventSerializationBinderTests
26+
{
27+
// Mirrors the production read settings on AzureTableOrchestrationHistoryEventEntity.
28+
static readonly JsonSerializerSettings ReadJsonSettings = new JsonSerializerSettings
29+
{
30+
TypeNameHandling = TypeNameHandling.Objects,
31+
SerializationBinder = new HistoryEventSerializationBinder()
32+
};
33+
34+
// Settings used to *produce* the JSON exactly as the production code does.
35+
static readonly JsonSerializerSettings WriteJsonSettings = new JsonSerializerSettings
36+
{
37+
TypeNameHandling = TypeNameHandling.Objects
38+
};
39+
40+
[TestMethod]
41+
public void RoundTripsExecutionStartedEventWithTags()
42+
{
43+
var original = new ExecutionStartedEvent(eventId: -1, input: "input")
44+
{
45+
OrchestrationInstance = new OrchestrationInstance
46+
{
47+
InstanceId = "instance-1",
48+
ExecutionId = "execution-1",
49+
},
50+
Name = "OrchestrationName",
51+
Version = "1.0",
52+
Tags = new Dictionary<string, string> { ["tag1"] = "value1" },
53+
};
54+
55+
string json = JsonConvert.SerializeObject(original, WriteJsonSettings);
56+
var deserialized = JsonConvert.DeserializeObject<HistoryEvent>(json, ReadJsonSettings);
57+
58+
Assert.IsInstanceOfType(deserialized, typeof(ExecutionStartedEvent));
59+
var deserializedStarted = (ExecutionStartedEvent)deserialized;
60+
Assert.AreEqual("OrchestrationName", deserializedStarted.Name);
61+
Assert.AreEqual("input", deserializedStarted.Input);
62+
Assert.AreEqual("value1", deserializedStarted.Tags["tag1"]);
63+
}
64+
65+
[TestMethod]
66+
public void AllowsAllConcreteHistoryEventTypes()
67+
{
68+
// Sanity check that every concrete HistoryEvent subclass declared in DurableTask.Core
69+
// is accepted by the binder's BindToType.
70+
var binder = new HistoryEventSerializationBinder();
71+
foreach (Type concreteType in HistoryEvent.KnownTypes())
72+
{
73+
Type bound = binder.BindToType(concreteType.Assembly.GetName().Name, concreteType.FullName);
74+
Assert.AreEqual(concreteType, bound);
75+
}
76+
}
77+
78+
[TestMethod]
79+
public void RejectsNonAllowlistedRootType()
80+
{
81+
// System.IO.FileInfo is a classic gadget-chain probe. The binder must reject it
82+
// even though the BCL would happily resolve it via Type.GetType.
83+
string json = "{\"$type\":\"System.IO.FileInfo, System.Private.CoreLib\",\"OriginalPath\":\"c:\\\\evil\"}";
84+
Assert.ThrowsException<JsonSerializationException>(
85+
() => JsonConvert.DeserializeObject<HistoryEvent>(json, ReadJsonSettings));
86+
}
87+
88+
[TestMethod]
89+
public void RejectsNonAllowlistedNestedType()
90+
{
91+
// Embed a malicious $type inside an otherwise valid ExecutionStartedEvent's Tags
92+
// member. The Tags member is IDictionary<string,string>, so the $type token is
93+
// honored by Newtonsoft.Json when reading. Anything other than
94+
// Dictionary<string,string> must be rejected.
95+
string json = "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\","
96+
+ "\"Tags\":{\"$type\":\"System.Collections.Generic.SortedDictionary`2[[System.String, System.Private.CoreLib],[System.String, System.Private.CoreLib]], System.Collections\"}}";
97+
Assert.ThrowsException<JsonSerializationException>(
98+
() => JsonConvert.DeserializeObject<HistoryEvent>(json, ReadJsonSettings));
99+
}
100+
101+
[TestMethod]
102+
public void AllowsDictionaryStringStringForTags()
103+
{
104+
// The exact concrete type Newtonsoft.Json emits for IDictionary<string,string> Tags.
105+
string json = "{\"$type\":\"DurableTask.Core.History.ExecutionStartedEvent, DurableTask.Core\","
106+
+ "\"Tags\":{\"$type\":\"System.Collections.Generic.Dictionary`2[[System.String, " + typeof(string).Assembly.GetName().Name + "],[System.String, " + typeof(string).Assembly.GetName().Name + "]], " + typeof(Dictionary<string, string>).Assembly.GetName().Name + "\","
107+
+ "\"tag1\":\"v1\"}}";
108+
109+
var deserialized = (ExecutionStartedEvent)JsonConvert.DeserializeObject<HistoryEvent>(json, ReadJsonSettings);
110+
Assert.AreEqual("v1", deserialized.Tags["tag1"]);
111+
}
112+
}
113+
}

src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ namespace DurableTask.ServiceBus.Tracking
1818
using System.Runtime.Serialization;
1919
using Azure.Data.Tables;
2020
using DurableTask.Core.History;
21-
using DurableTask.Core.Serializing;
2221
using Newtonsoft.Json;
2322

2423
/// <summary>
@@ -35,7 +34,7 @@ internal class AzureTableOrchestrationHistoryEventEntity : AzureTableCompositeTa
3534
private static readonly JsonSerializerSettings ReadJsonSettings = new JsonSerializerSettings
3635
{
3736
TypeNameHandling = TypeNameHandling.Objects,
38-
SerializationBinder = new PackageUpgradeSerializationBinder()
37+
SerializationBinder = new HistoryEventSerializationBinder()
3938
};
4039

4140
/// <summary>
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// ----------------------------------------------------------------------------------
2+
// Copyright Microsoft Corporation
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
// http://www.apache.org/licenses/LICENSE-2.0
7+
// Unless required by applicable law or agreed to in writing, software
8+
// distributed under the License is distributed on an "AS IS" BASIS,
9+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
// See the License for the specific language governing permissions and
11+
// limitations under the License.
12+
// ----------------------------------------------------------------------------------
13+
14+
namespace DurableTask.ServiceBus.Tracking
15+
{
16+
using System;
17+
using System.Collections.Generic;
18+
using System.Reflection;
19+
using DurableTask.Core.History;
20+
using DurableTask.Core.Serializing;
21+
using Newtonsoft.Json;
22+
23+
/// <summary>
24+
/// Strict <see cref="Newtonsoft.Json.Serialization.ISerializationBinder"/> used when deserializing
25+
/// orchestration history events stored in Azure Table tracking. Only types that the framework
26+
/// itself emits when serializing a <see cref="HistoryEvent"/> are accepted; any other
27+
/// <c>$type</c> token is rejected with a <see cref="JsonSerializationException"/>.
28+
/// </summary>
29+
/// <remarks>
30+
/// This binder is intentionally restrictive: the history event JSON written by DTFx never
31+
/// contains polymorphic customer types (customer payloads such as <c>Input</c>/<c>Result</c>/
32+
/// <c>Reason</c>/<c>Details</c> are pre-serialized strings on the <see cref="HistoryEvent"/>
33+
/// subtree and are opaque to this serializer). Restricting the resolvable type set defends
34+
/// against unsafe-deserialization gadget chains if a malicious <c>$type</c> were ever written
35+
/// into the tracking table by an attacker with write access to the Storage account.
36+
/// </remarks>
37+
internal sealed class HistoryEventSerializationBinder : PackageUpgradeSerializationBinder
38+
{
39+
static readonly Assembly DurableTaskCoreAssembly = typeof(HistoryEvent).Assembly;
40+
41+
/// <inheritdoc />
42+
public override Type BindToType(string assemblyName, string typeName)
43+
{
44+
Type resolved = base.BindToType(assemblyName, typeName);
45+
46+
if (resolved == null || !IsAllowed(resolved))
47+
{
48+
throw new JsonSerializationException(
49+
$"Type '{typeName}' from assembly '{assemblyName}' is not permitted by the orchestration history serialization binder.");
50+
}
51+
52+
return resolved;
53+
}
54+
55+
static bool IsAllowed(Type type)
56+
{
57+
// Allow any type defined in DurableTask.Core (HistoryEvent subclasses, OrchestrationInstance,
58+
// ParentInstance, FailureDetails, OrchestrationExecutionContext, etc.), plus
59+
// Dictionary<string, string> to round-trip the IDictionary<string, string> Tags members
60+
// declared on ExecutionStartedEvent / SubOrchestrationInstanceCreatedEvent /
61+
// TaskScheduledEvent (Newtonsoft.Json emits a $type for these because the static
62+
// declared type is an interface).
63+
return type.Assembly == DurableTaskCoreAssembly
64+
|| type == typeof(Dictionary<string, string>);
65+
}
66+
}
67+
}

0 commit comments

Comments
 (0)