Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Guard against (de)serializing pointers & ref structs #34777

Merged
merged 12 commits into from
Apr 27, 2020
7 changes: 5 additions & 2 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -506,13 +506,16 @@
<data name="BufferMaximumSizeExceeded" xml:space="preserve">
<value>Cannot allocate a buffer of size {0}.</value>
</data>
<data name="CannotSerializeOpenGeneric" xml:space="preserve">
<value>The type '{0}' has generic parameters that have not been replaced by specific types, which is not valid during serialization or deserialization.</value>
<data name="CannotSerializeInvalidType" xml:space="preserve">
<value>The type '{0}' is invalid for serialization or deserialization because it is a pointer type, is a ref struct, or contains generic parameters that have not been replaced by specific types.</value>
</data>
<data name="SerializeTypeInstanceNotSupported" xml:space="preserve">
<value>Serialization and deserialization of 'System.Type' instances are not supported and should be avoided since they can lead to security issues.</value>
</data>
<data name="JsonIncludeOnNonPublicInvalid" xml:space="preserve">
<value>The non-public property '{0}' on type '{1}' is annotated with 'JsonIncludeAttribute' which is invalid.</value>
</data>
<data name="CannotSerializeInvalidMember" xml:space="preserve">
<value>The type '{0}' of property '{1}' on type '{2}' is invalid for serialization or deserialization because it is a pointer type, is a ref struct, or contains generic parameters that have not been replaced by specific types.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text.Json.Serialization;

namespace System.Text.Json
Expand Down Expand Up @@ -352,6 +353,7 @@ public static JsonConverter GetConverter(
JsonSerializerOptions options)
{
Debug.Assert(type != null);
ValidateType(type, parentClassType, propertyInfo, options);

JsonConverter converter = options.DetermineConverter(parentClassType, type, propertyInfo)!;

Expand Down Expand Up @@ -395,7 +397,46 @@ public static JsonConverter GetConverter(
}
}

Debug.Assert(!IsInvalidForSerialization(runtimeType));

return converter;
}

private static void ValidateType(Type type, Type? parentClassType, PropertyInfo? propertyInfo, JsonSerializerOptions options)
{
if (!options.TypeIsCached(type) && IsInvalidForSerialization(type))
{
ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(type, parentClassType, propertyInfo);
}
}

private static bool IsInvalidForSerialization(Type type)
{
return type.IsPointer || IsByRefLike(type) || type.ContainsGenericParameters;
}

private static bool IsByRefLike(Type type)
{
#if BUILDING_INBOX_LIBRARY
return type.IsByRefLike;
#else
if (!type.IsValueType)
{
return false;
}

object[] attributes = type.GetCustomAttributes(inherit: false);

for (int i = 0; i < attributes.Length; i++)
{
if (attributes[i].GetType().FullName == "System.Runtime.CompilerServices.IsByRefLikeAttribute")
layomia marked this conversation as resolved.
Show resolved Hide resolved
{
return true;
}
}

return false;
layomia marked this conversation as resolved.
Show resolved Hide resolved
#endif
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics;
using System.Text.Json.Serialization;
using System.Text.Encodings.Web;
using System.Reflection;

namespace System.Text.Json
{
Expand Down Expand Up @@ -377,17 +378,17 @@ internal JsonClassInfo GetOrAddClass(Type type)
// https://github.com/dotnet/runtime/issues/32357
if (!_classes.TryGetValue(type, out JsonClassInfo? result))
{
if (type.ContainsGenericParameters)
{
ThrowHelper.ThrowInvalidOperationException_CannotSerializeOpenGeneric(type);
}

result = _classes.GetOrAdd(type, new JsonClassInfo(type, this));
}

return result;
}

internal bool TypeIsCached(Type type)
{
return _classes.ContainsKey(type);
}

internal JsonReaderOptions GetReaderOptions()
{
return new JsonReaderOptions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,16 @@ public static void ThrowJsonException(string? message = null)

[DoesNotReturn]
[MethodImpl(MethodImplOptions.NoInlining)]
public static void ThrowInvalidOperationException_CannotSerializeOpenGeneric(Type type)
public static void ThrowInvalidOperationException_CannotSerializeInvalidType(Type type, Type? parentClassType, PropertyInfo? propertyInfo)
layomia marked this conversation as resolved.
Show resolved Hide resolved
{
throw new InvalidOperationException(SR.Format(SR.CannotSerializeOpenGeneric, type));
if (parentClassType == null)
{
Debug.Assert(propertyInfo == null);
throw new InvalidOperationException(SR.Format(SR.CannotSerializeInvalidType, type));
}

Debug.Assert(propertyInfo != null);
throw new InvalidOperationException(SR.Format(SR.CannotSerializeInvalidMember, type, propertyInfo.Name, parentClassType));
}

[DoesNotReturn]
Expand Down
167 changes: 167 additions & 0 deletions src/libraries/System.Text.Json/tests/Serialization/InvalidTypeTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using Xunit;

namespace System.Text.Json.Serialization.Tests
{
public class InvalidTypeTests_Span : InvalidTypeTests
{
public InvalidTypeTests_Span() : base(SerializationWrapper.SpanSerializer) { }
}

public class InvalidTypeTests_String : InvalidTypeTests
{
public InvalidTypeTests_String() : base(SerializationWrapper.StringSerializer) { }
}

public class InvalidTypeTests_Stream : InvalidTypeTests
{
public InvalidTypeTests_Stream() : base(SerializationWrapper.StreamSerializer) { }
}

public class InvalidTypeTests_StreamWithSmallBuffer : InvalidTypeTests
{
public InvalidTypeTests_StreamWithSmallBuffer() : base(SerializationWrapper.StreamSerializerWithSmallBuffer) { }
}

public class InvalidTypeTests_Writer : InvalidTypeTests
{
public InvalidTypeTests_Writer() : base(SerializationWrapper.WriterSerializer) { }
}

public abstract class InvalidTypeTests
{
private SerializationWrapper Serializer { get; }

public InvalidTypeTests(SerializationWrapper serializer)
{
Serializer = serializer;
}

[Theory]
[MemberData(nameof(OpenGenericTypes))]
[MemberData(nameof(RefStructTypes))]
[MemberData(nameof(PointerTypes))]
public void DeserializeInvalidType(Type type)
layomia marked this conversation as resolved.
Show resolved Hide resolved
{
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize("", type));
Assert.Contains(type.ToString(), ex.ToString());
}

[Theory]
[MemberData(nameof(TypesWithInvalidMembers_WithMembers))]
public void TypeWithInvalidMember(Type classType, Type invalidMemberType, string invalidMemberName)
{
static void ValidateException(InvalidOperationException ex, Type classType, Type invalidMemberType, string invalidMemberName)
{
string exAsStr = ex.ToString();
Assert.Contains(invalidMemberType.ToString(), exAsStr);
Assert.Contains(invalidMemberName, exAsStr);
Assert.Contains(classType.ToString(), exAsStr);
}

object obj = Activator.CreateInstance(classType);
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => Serializer.Serialize(obj, classType));
ValidateException(ex, classType, invalidMemberType, invalidMemberName);

ex = Assert.Throws<InvalidOperationException>(() => Serializer.Serialize(null, classType));
ValidateException(ex, classType, invalidMemberType, invalidMemberName);

ex = Assert.Throws<InvalidOperationException>(() => JsonSerializer.Deserialize("", classType));
ValidateException(ex, classType, invalidMemberType, invalidMemberName);
}

[Theory]
[MemberData(nameof(OpenGenericTypes_ToSerialize))]
public void SerializeOpenGeneric(Type type)
{
object obj;

if (type.GetGenericArguments().Length == 1)
{
obj = Activator.CreateInstance(type.MakeGenericType(typeof(int)));
}
else
{
obj = Activator.CreateInstance(type.MakeGenericType(typeof(string), typeof(int)));
}

Assert.Throws<ArgumentException>(() => Serializer.Serialize(obj, type));
}

[Theory]
[MemberData(nameof(OpenGenericTypes))]
public void SerializeInvalidTypes_NullValue(Type type)
{
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => Serializer.Serialize(null, type));
Assert.Contains(type.ToString(), ex.ToString());
}

[Fact]
public void SerializeOpenGeneric_NullableOfT()
{
Type openNullableType = typeof(Nullable<>);
object obj = Activator.CreateInstance(openNullableType.MakeGenericType(typeof(int)));

InvalidOperationException ex = Assert.Throws<InvalidOperationException>(() => Serializer.Serialize(obj, openNullableType));
Assert.Contains(openNullableType.ToString(), ex.ToString());
}

private class Test<T> { }

private static IEnumerable<object[]> OpenGenericTypes()
{
yield return new object[] { typeof(Test<>) };
yield return new object[] { typeof(Nullable<>) };
yield return new object[] { typeof(IList<>) };
yield return new object[] { typeof(List<>) };
yield return new object[] { typeof(List<>).MakeGenericType(typeof(Test<>)) };
yield return new object[] { typeof(Test<>).MakeGenericType(typeof(List<>)) };
yield return new object[] { typeof(Dictionary<,>) };
yield return new object[] { typeof(Dictionary<,>).MakeGenericType(typeof(string), typeof(Nullable<>)) };
yield return new object[] { typeof(Dictionary<,>).MakeGenericType(typeof(Nullable<>), typeof(string)) };
}

private static IEnumerable<object[]> OpenGenericTypes_ToSerialize()
{
yield return new object[] { typeof(Test<>) };
yield return new object[] { typeof(List<>) };
yield return new object[] { typeof(Dictionary<,>) };
}

private static IEnumerable<object[]> RefStructTypes()
{
yield return new object[] { typeof(Span<int>) };
yield return new object[] { typeof(Utf8JsonReader) };
yield return new object[] { typeof(MyRefStruct) };
}

private static readonly Type s_intPtrType = typeof(int*); // Unsafe code may not appear in iterators.

private static IEnumerable<object[]> PointerTypes()
{
yield return new object[] { s_intPtrType };
}

// Instances of the types of the invalid members cannot be passed directly
// to the serializer on serialization due to type constraints,
// e.g. int* can't be boxed and passed to the non-generic overload,
// and typeof(int*) can't be a generic parameter to the generic overload.
private static IEnumerable<object[]> TypesWithInvalidMembers_WithMembers()
{
yield return new object[] { typeof(Memory<byte>), typeof(Span<byte>), "Span" }; // Contains Span<byte> property.

yield return new object[] { typeof(ClassWithIntPtr), s_intPtrType, "IntPtr" };
}

private class ClassWithIntPtr
{
public unsafe int* IntPtr { get; }
}

private ref struct MyRefStruct { }
}
}
Loading