2
\$\begingroup\$

I use this method to get either Customer or Account. The server will determine which type it is.

The response will have a property "Type": "Customer" or "Type": "Account".

I first deserialize to Client (supertype) to check the Type property. Then deserialize to either Customer or Account.

public async Task<Models.Entities.Client> GetClient(int clientId)
{
    var getClientRequest = new RestRequest("client/details", Method.GET);
    getClientRequest.AddQueryParameter("ClientId", clientId);

    var jsonResponse = await _requestService.DoRequest(getClientRequest);

    var client = JsonConvert.DeserializeObject<Models.Entities.Client>(jsonResponse);

    switch (client.Type)
    {
        case ClientKind.Account:
            var account = JsonConvert.DeserializeObject<Account>(jsonResponse)
            return account;
        default:
            var customer = JsonConvert.DeserializeObject<Customer>(jsonResponse);
            return customer;
    }
}

Example responses:

{
    "ClientId": 1,
    "Type": "Account",
    "Name": "Company Inc."
}
{
    "ClientId": 2,
    "Type": "Customer",
    "Name":
    {
        "First": "John",
        "Last": "Smith"
    },
    "DateOfBirth": "1960-12-01"
}

DTOs:

public class Client
{
    public int ClientId { get; set; }
    public ClientType Type { get; set; }
}

public class Account : Client
{
    public string Name { get; set; }
}

public class Customer : Client
{
    public PersonalName Name { get; set; }
    public DateTime DateOfBirth { get; set; }
}
\$\endgroup\$
1
\$\begingroup\$

Your code could be rewritten:

var settings = new JsonSerializerSettings { 
     TypeNameHandling = TypeNameHandling.All,
     SerializationBinder = knownTypesBinder // <- see security risk below
};
var client = JsonConvert.DeserializeObject<Models.Entities.Client>(jsonResponse);
return client; // <- strongly-typed
  • Make sure both server and client use the settings
  • If you have declared public ClientType Type { get; set; } just to enable two-phase serialisation (base entity - concrete entity), you should remove it from the code.
  • The two-phase serialisation hack can be replaced with a strongly-typed serialisation using TypeNameHandling = TypeNameHandling.All. Example

As suggested in the comments, we need to address the security aspect also. Hence, knownTypesBinder is used to mitigate a security risk.

// based on https://www.newtonsoft.com/json/help/html/SerializeSerializationBinder.htm
var knownTypesBinder = new KnownTypesBinder
{
    KnownTypes = new List<Type> { typeof(Customer), typeof(Account) }
};

public class KnownTypesBinder : ISerializationBinder
{
    public IList<Type> KnownTypes { get; set; }

    public Type BindToType(string assemblyName, string typeName)
    {
        return KnownTypes.SingleOrDefault(t => t.Name == typeName);
    }

    public void BindToName(Type serializedType, out string assemblyName, 
        out string typeName)
    {
        assemblyName = null;
        typeName = serializedType.Name;
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ While none of the statements in this answer are false, the omission of any mention of the massive security flaws of the example code is a big problem. If using TypeNameHandling you must also use SerializationBinder and white-list the types which can be loaded. \$\endgroup\$ – Peter Taylor Jun 13 at 7:18
  • 1
    \$\begingroup\$ @PeterTaylor what kind of massive security flaws do you mean and where did you get this info that the SerializationBinder must be used? It's not like the serializer throws when you don't... the examples on newtonsoft don't mention it. \$\endgroup\$ – t3chb0t Jun 13 at 7:22
  • 1
    \$\begingroup\$ @t3chb0t, I followed the link in the answer, read the comments, and followed the links there. In brief, if you don't white-list then an attacker can craft a serialised class which calls the public constructor of any class which can be loaded and then calls any sequence of public property setters on it. The precise implications depend on what libraries are available, but you can e.g. cause side-effects on the filesystem by creating a FileInfo and then manipulating properties like IsReadOnly. The worst case, seen in a similar Java vuln due to an Apache lib, is execution of arbitrary code. \$\endgroup\$ – Peter Taylor Jun 13 at 8:01
  • \$\begingroup\$ @PeterTaylor It does make sense to me to consider the security implications. As a comparison, System.ServiceModel has foreseen this in their API: docs.microsoft.com/en-us/dotnet/framework/wcf/feature-details/… \$\endgroup\$ – dfhwze Jun 13 at 8:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.