Skip to content

Commit 4ced30d

Browse files
mcatanzaritimcatanzariti
committed
fix Security Issues with Polymorphic support in serialization
* BREAKING CHANGE: removed DefaultDiscriminatorConvention * BREAKING CHANGE: renamed AttributeBasedDiscriminatorConvention<T> into DefaultDiscriminatorConvention<T> * check if declared type is assignable from discriminator type before instantiation * fix #107
1 parent 1719093 commit 4ced30d

File tree

13 files changed

+47
-136
lines changed

13 files changed

+47
-136
lines changed

‎src/Dahomey.Cbor.Tests/CreatorMappingTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,6 @@ public Person(int id, string name)
224224
public void InheritedClassWithConstructor()
225225
{
226226
CborOptions options = new CborOptions();
227-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
228227
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(Person));
229228

230229
const string hexBuffer = "A3625F7466506572736F6E6249640C644E616D6563466F6F";

‎src/Dahomey.Cbor.Tests/DiscriminatorTests.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ static DiscriminatorTests()
1919
public void ReadPolymorphicObject()
2020
{
2121
CborOptions options = new CborOptions();
22-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
2322
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(NameObject));
2423

2524
const string hexBuffer = "A16A426173654F626A656374A3625F746A4E616D654F626A656374644E616D6563666F6F62496401";
@@ -31,20 +30,22 @@ public void ReadPolymorphicObject()
3130
Assert.Equal(1, obj.BaseObject.Id);
3231
}
3332

33+
[CborDiscriminator("OtherObject")]
34+
public class OtherObject
35+
{
36+
}
37+
3438
[Fact]
35-
public void ReadPolymorphicObjectWithDefaultDiscriminatorConvention()
39+
public void ReadNonAssignablePolymorphicObject()
3640
{
37-
const string hexBuffer = "A2625F7478374461686F6D65792E43626F722E54657374732E426173654F626A656374486F6C6465722C204461686F6D65792E43626F722E54657374736A426173654F626A656374A3625F746A4E616D654F626A656374644E616D6563666F6F62496401";
41+
const string hexBuffer = "A16A426173654F626A656374A1625F746B4F746865724F626A656374"; // {"BaseObject": {"_t": "OtherObject"}}
3842

3943
CborOptions options = new CborOptions();
4044
DiscriminatorConventionRegistry registry = options.Registry.DiscriminatorConventionRegistry;
41-
registry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
4245
registry.RegisterType<NameObject>();
46+
registry.RegisterType<OtherObject>();
4347

44-
object obj = Helper.Read<object>(hexBuffer, options);
45-
46-
Assert.NotNull(obj);
47-
Assert.IsType<BaseObjectHolder>(obj);
48+
Assert.ThrowsAny<CborException>(() => Helper.Read<BaseObjectHolder>(hexBuffer, options));
4849
}
4950

5051
[Theory]
@@ -55,7 +56,6 @@ public void ReadPolymorphicObjectWithDefaultDiscriminatorConvention()
5556
public void WritePolymorphicObject(CborDiscriminatorPolicy discriminatorPolicy, string hexBuffer)
5657
{
5758
CborOptions options = new CborOptions();
58-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
5959
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(NameObject));
6060
options.Registry.ObjectMappingRegistry.Register<NameObject>(om =>
6161
{

‎src/Dahomey.Cbor.Tests/Issues/Issue0020.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ public class Orange : Fruit
4848
public void TestWrite()
4949
{
5050
CborOptions options = new CborOptions();
51-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
5251
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(Apple));
5352
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(Orange));
5453

@@ -62,7 +61,6 @@ public void TestWrite()
6261
public void TestRead()
6362
{
6463
CborOptions options = new CborOptions();
65-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
6664
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(Apple));
6765
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(Orange));
6866

‎src/Dahomey.Cbor.Tests/Issues/Issue0027.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,13 @@ public class Car
1515
[Fact]
1616
public void Test()
1717
{
18-
CborOptions options = new CborOptions();
19-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
20-
2118
var car = new Car()
2219
{
2320
Description = "n"
2421
};
2522

2623
string hexBuffer = "A2625F7471736F6D656469736372696D696E61746F726B4465736372697074696F6E616E";
27-
Helper.TestWrite(car, hexBuffer, null, options);
24+
Helper.TestWrite(car, hexBuffer);
2825
}
2926
}
3027
}

‎src/Dahomey.Cbor.Tests/Issues/Issue0029.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ public class DummyMessageParticle : DummyParticle
2121
public void Test()
2222
{
2323
CborOptions options = new CborOptions();
24-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
2524
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(DummyMessageParticle));
2625

2726
// {"nonce": 2181035975144481159, "_t": "radix.particles.message"}

‎src/Dahomey.Cbor.Tests/Issues/Issue0046.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public class Atom
2121
public void Test()
2222
{
2323
CborOptions options = new CborOptions();
24-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry, "serializer"));
24+
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new DefaultDiscriminatorConvention<string>(options.Registry, "serializer"));
2525
options.Registry.ObjectMappingRegistry.Register<Atom>(objectMapping =>
2626
{
2727
objectMapping.AutoMap();

‎src/Dahomey.Cbor.Tests/Issues/Issue0091.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ public record class Dog(string Color, DateTime Timestamp) : Animal(Timestamp)
2323
public void TestReadWrite()
2424
{
2525
CborOptions options = new CborOptions();
26-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
2726
options.Registry.DiscriminatorConventionRegistry.RegisterType(typeof(Dog));
2827

2928
Dog dog = new("black", DateTime.Parse("2022-10-24T14:05:08Z", null, DateTimeStyles.RoundtripKind));

‎src/Dahomey.Cbor.Tests/ObjectMappingTests.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ public class InheritedObject : BaseObject
172172
public void ReadWithDiscriminator()
173173
{
174174
CborOptions options = new CborOptions();
175-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
176175
options.Registry.ObjectMappingRegistry.Register<InheritedObject>(objectMapping =>
177176
objectMapping
178177
.AutoMap()
@@ -192,7 +191,6 @@ public void ReadWithDiscriminator()
192191
public void WriteWithDiscriminator()
193192
{
194193
CborOptions options = new CborOptions();
195-
options.Registry.DiscriminatorConventionRegistry.RegisterConvention(new AttributeBasedDiscriminatorConvention<string>(options.Registry));
196194
options.Registry.ObjectMappingRegistry.Register<InheritedObject>(objectMapping =>
197195
objectMapping
198196
.SetDiscriminator("inherited")

‎src/Dahomey.Cbor/Serialization/Conventions/AttributeBasedDiscriminatorConvention.cs‎

Lines changed: 0 additions & 67 deletions
This file was deleted.

‎src/Dahomey.Cbor/Serialization/Conventions/DefaultDiscriminatorConvention.cs‎

Lines changed: 28 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,19 @@
1-
using Dahomey.Cbor.Util;
1+
using Dahomey.Cbor.Serialization.Converters;
2+
using Dahomey.Cbor.Serialization.Converters.Mappings;
3+
using Dahomey.Cbor.Util;
24
using System;
35
using System.Collections.Concurrent;
4-
using System.Reflection;
5-
using System.Text;
66

77
namespace Dahomey.Cbor.Serialization.Conventions
88
{
9-
public class DefaultDiscriminatorConvention : IDiscriminatorConvention
9+
public class DefaultDiscriminatorConvention<T> : IDiscriminatorConvention
10+
where T : notnull
1011
{
1112
private readonly SerializationRegistry _serializationRegistry;
1213
private readonly ReadOnlyMemory<byte> _memberName;
13-
private readonly ConcurrentDictionary<string, Type> _typesByDiscriminator = new ConcurrentDictionary<string, Type>();
14-
private readonly ConcurrentDictionary<Type, string> _discriminatorsByType = new ConcurrentDictionary<Type, string>();
14+
private readonly ConcurrentDictionary<T, Type> _typesByDiscriminator = new();
15+
private readonly ConcurrentDictionary<Type, T> _discriminatorsByType = new();
16+
private readonly ICborConverter<T> _converter;
1517

1618
public ReadOnlySpan<byte> MemberName => _memberName.Span;
1719

@@ -24,69 +26,49 @@ public DefaultDiscriminatorConvention(SerializationRegistry serializationRegistr
2426
{
2527
_serializationRegistry = serializationRegistry;
2628
_memberName = memberName.AsBinaryMemory();
29+
_converter = serializationRegistry.ConverterRegistry.Lookup<T>();
2730
}
2831

2932

3033
public bool TryRegisterType(Type type)
3134
{
35+
IObjectMapping objectMapping = _serializationRegistry.ObjectMappingRegistry.Lookup(type);
36+
37+
if (objectMapping.Discriminator == null || objectMapping.Discriminator is not T discriminator)
38+
{
39+
return false;
40+
}
41+
42+
_discriminatorsByType.TryAdd(type, discriminator);
43+
_typesByDiscriminator.TryAdd(discriminator, type);
3244
return true;
3345
}
3446

3547
public Type ReadDiscriminator(ref CborReader reader)
3648
{
37-
string? discriminator = reader.ReadString();
49+
T discriminator = _converter.Read(ref reader);
3850

3951
if (discriminator == null)
4052
{
41-
throw reader.BuildException("Discrimnator cannot be null or empty");
53+
throw new CborException("Null discriminator");
54+
}
55+
56+
if (!_typesByDiscriminator.TryGetValue(discriminator, out Type? type))
57+
{
58+
throw new CborException($"Unknown type discriminator: {discriminator}");
4259
}
4360

44-
Type type = _typesByDiscriminator.GetOrAdd(discriminator, NameToType);
4561
return type;
4662
}
4763

4864
public void WriteDiscriminator(ref CborWriter writer, Type actualType)
4965
{
50-
string discriminator = _discriminatorsByType.GetOrAdd(actualType, TypeToName);
51-
writer.WriteString(discriminator);
52-
}
53-
54-
private string TypeToName(Type type)
55-
{
56-
return type.FullName + ", " + type.Assembly.GetName().Name;
57-
}
58-
59-
private Type NameToType(string name)
60-
{
61-
string[] parts = name.Split(new[] { ',', ' ' }, StringSplitOptions.RemoveEmptyEntries);
62-
63-
string? assemblyName;
64-
string typeName;
65-
66-
switch (parts.Length)
67-
{
68-
case 1:
69-
typeName = parts[0];
70-
assemblyName = null;
71-
break;
72-
73-
case 2:
74-
typeName = parts[0];
75-
assemblyName = parts[1];
76-
break;
77-
78-
default:
79-
throw new CborException($"Invalid discriminator {name}");
80-
81-
}
82-
83-
if (!string.IsNullOrEmpty(assemblyName))
66+
if (!_discriminatorsByType.TryGetValue(actualType, out T? discriminator))
8467
{
85-
Assembly assembly = Assembly.Load(assemblyName);
86-
return assembly.GetType(typeName) ?? throw new CborException($"Cannot get type from {name}");
68+
throw new CborException($"Unknown discriminator for type: {actualType}");
8769
}
8870

89-
return Type.GetType(typeName) ?? throw new CborException($"Cannot get type from {name}");
71+
_converter.Write(ref writer, discriminator);
9072
}
9173
}
9274
}

0 commit comments

Comments
 (0)