Search code examples
c#validationsecurityjson.netjson-deserialization

Is it possible to prevent the Json.Net TypeNameHandling vulnerability with a marker for trusted types?


I was reading about the vulnerability of deserializing types with Json.Net using a setting different from TypeNameHandling.None. The Json.Net docs recommend implementing a custom SerializationBinder. A simple example of a custom binder that checks types against a list of known types is given here.

While this solution certainly works, the set of known types is not fixed in my scenario, since the application has to support extensions, which might define their own data classes. One solution would be to extend the known type list during the registration of an extension, however, I had a second approach in mind, that I'd like to verify:

I want to define a common interface for trusted types:

(suggested by dbc: A custom attribute could be used instead of a marker interface.)

public interface ITrustedType { }

Then, any types that need to be (de-)serialized by the application or an extension, can implement this interface:

public class Car : ITrustedType
{
   public IList<Passenger> Passengers { get; set; }
   // ...
}

public class Passenger : ITrustedType
{
   // ...
}

The custom binder does not check against a list of types, but instead, it will make sure that each (de-)serialized type is implementing the trusted type interface, applying the default behavior in case it does or failing if it doesn't.

In the case of an enumerable type, it would make sure that the generic type parameters are trusted types.

This is a simple implementation with which I tested so far:

public class TrustedTypesBinder : DefaultSerializationBinder
{
    static bool IsCollectionType(Type type)
    {
        return type.GetInterfaces().Any(s => s.Namespace == "System.Collections.Generic" && s.Name.StartsWith("IEnumerable`"));
    }

    static public bool IsTrustedType(Type type)
    {
        if (IsCollectionType(type))
        {
            return type.GenericTypeArguments.All(t => IsTrustedType(t));
        }
        return typeof(ITrustedType).IsAssignableFrom(type);
    }

    public override Type BindToType(string assemblyName, string typeName)
    {
        Type type = base.BindToType(assemblyName, typeName);
        if (!IsTrustedType(type))
            throw new JsonSerializationException(typeName + " is not a trusted type.");
        return type;
    }

    public override void BindToName(Type serializedType, out string assemblyName, out string typeName)
    {
        if (!IsTrustedType(serializedType))
            throw new JsonSerializationException(serializedType.FullName + " is not a trusted type.");
        base.BindToName(serializedType, out assemblyName, out typeName);
    }
}

I suspect this solution to be safe since no type from outside the app or extension assembly will implement this interface, so no attack gadgets can be constructed from a malicious JSON input.

Am I missing any attack vectors? Do you know a way in which this could be compromised?

Edit: I am aware, that it is possible for anyone to implement this ITrustedType interface and give it some arbitrary behavior. As far as I know, this should not be relevant in the context of (de-)serialization at runtime. The app itself, as well as the extensions, are coming from trusted sources, so of course, implemented types are controlled and it will not be possible to add any new types into the published application binaries for an attacker.

This question is specifically about instantiating types that are present at runtime and could be used for malicious purposes.

Edit 2: Changed ICollection to IEnumerable, based on Ben Voigts suggestion.


Solution

  • When you encounter a type that isn't marked, it is not sufficient to check its generic type arguments, you need to check the type of every public property and every parameter of a public constructor, because these are the serialization footprint.

    For example, you really do not want to allow deserialization of a System.Data.TypedTableBase<T> even if T is safe, because it has public properties that allow configuring database access.