2

I've been struggling with this for a little while now, I'm trying to bind a client-side JSON post to a model in ASP.NET Core 2 MVC but I can't get the values to take. The JSON data I'm posting is below.

{
    "$type": "Namespace.ApplicationLoggingModel, Projectname.Web",
    "Test": "works"
}

...and the model:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;

namespace Namespace.Models
{
    public class ApplicationLoggingModel
    {
        public string Test { get; set; }
    }
}

The controller action seems to know it's ApplicationLoggingModel class, but it won't bind the test data. I've also tried the JSON below but that doesn't work either.

{
    "$Type": "Namespace.ApplicationLoggingModel, Projectname.Web",
    "$Values": [{
        "$Type": "System.String, mscorlib",
        "Test": true
    }]
}

I've also tried "$type" and "$values" but it doesn't appear to be case sensitive, so I'm a little bit stuck. Is this even possible in .NET Core and with model binding? I've had to change the project and namespace names so let me know if you need any more information.

UPDATE: I've added the controller action below, having tried regular model binding it seems like the $type isn't working as I've since added the [FromBody] tag to get the regular model binding to work. The model is now coming through null.

[HttpPost("Save/{id}")]
public ActionResult Save(Guid id, [FromBody]ApplicationLoggingModel model)
{
    return RedirectToAction("Entities", Guid.Empty);
}

EDIT: as dbc rightly pointed out, my ApplicationLoggingModel isn't polymorphic and therefore doesn't need TypeNameHandling - ApplicationLoggingModel will contain an array of polymorphic Models, I've basically taken a step back here to try and get one this working before I can implement it on the other models.

9
  • Since your ApplicationLoggingModel isn't polymorphic why do you need TypeNameHandling at all? Doing so can introduce security risks, see e.g. How to configure Json.NET to create a vulnerable web API and TypeNameHandling caution in Newtonsoft Json. Commented Mar 23, 2018 at 17:17
  • It will contain an array of polymorphic Models, I've basically taken a step back here to try and get one component working before I can implement it on the other models. I'll edit my post to point that out. Commented Mar 23, 2018 at 17:20
  • 1
    Then, oddly enough, the previously-mentioned How to configure Json.NET to create a vulnerable web API seems to show how to make it work. In that example there is a polymorphic property instead of a polymorphic root model. Also, you need to configure options.SerializerSettings.TypeNameHandling during startup. Commented Mar 23, 2018 at 17:23
  • By the way, do the models in your array of polymorphic Models derive from some common base class or interface? Commented Mar 23, 2018 at 17:28
  • They derive from an abstract class, I haven't got as far as to see if that part works yet, it was written by another developer. Also thanks for the article, adding options.SerializerSettings.TypeNameHandling = TypeNameHandling.All to AddJsonOptions has fixed the issue. I'll have to address the security issues as I'm pretty sure this functionality isn't something I can get around Commented Mar 23, 2018 at 17:32

1 Answer 1

1

As shown in How to configure Json.NET to create a vulnerable web API, you can enable TypeNameHandling during deserialization and model binding throughout your entire object graph by doing

services.AddMvc().AddJsonOptions(options =>
{
    options.SerializerSettings.TypeNameHandling = TypeNameHandling.All;
});

in Startup.cs.

However, doing this can introduce security risks as described in that very article as well as TypeNameHandling caution in Newtonsoft Json. As such I would not recommend this solution unless you create a custom ISerializationBinder to filter out unwanted or unexpected types.

As an alternative to this risky solution, if you only need to make your list of root models be polymorphic, the following approach can be used:

  1. Derive all of your polymorphic models from some common base class or interface defined in your application (i.e. not some system type such as CollectionBase or INotifyPropertyChanged).

  2. Define a container DTO with single property of type List<T> where T is your common base type.

  3. Mark that property with [JsonProperty(ItemTypeNameHandling = TypeNameHandling.Auto)].

  4. Do not set options.SerializerSettings.TypeNameHandling = TypeNameHandling.All; in startup.

To see how this works in practice, say you have the following model type hierarchy:

public abstract class ModelBase
{
}

public class ApplicationLoggingModel : ModelBase
{
    public string Test { get; set; }
}

public class AnotherModel : ModelBase
{
    public string AnotherTest { get; set; }
}

Then define your root DTO as follows:

public class ModelBaseCollectionDTO
{
    [JsonProperty(ItemTypeNameHandling = TypeNameHandling.Auto)]
    public List<ModelBase> Models { get; set; }
}

Then if you construct an instance of it as follows and serialize to JSON:

var dto = new ModelBaseCollectionDTO
{
    Models = new List<ModelBase>()
    {
        new ApplicationLoggingModel { Test = "test value" },
        new AnotherModel { AnotherTest = "another test value" },
    },
};

var json = JsonConvert.SerializeObject(dto, Formatting.Indented);

The following JSON is generated:

{
  "Models": [
    {
      "$type": "Namespace.Models.ApplicationLoggingModel, TestApp",
      "Test": "test value"
    },
    {
      "$type": "Namespace.Models.AnotherModel, TestApp",
      "AnotherTest": "another test value"
    }
  ]
}

This can then be deserialized back into a ModelBaseCollectionDTO without loss of type information and without needing to set ItemTypeNameHandling globally:

var dto2 = JsonConvert.DeserializeObject<ModelBaseCollectionDTO>(json);

Sample working fiddle.

However, if I attempt the attack shown in How to configure Json.NET to create a vulnerable web API as follows:

try
{
    File.WriteAllText("rce-test.txt", "");
    var badJson = JToken.FromObject(
        new
        {
            Models = new object[] 
            {
                new FileInfo("rce-test.txt") { IsReadOnly = false },
            }
        },
        JsonSerializer.CreateDefault(new JsonSerializerSettings { TypeNameHandling = TypeNameHandling.Auto, Formatting = Formatting.Indented }));
    ((JObject)badJson["Models"][0])["IsReadOnly"] = true;
    Console.WriteLine("Attempting to deserialize attack JSON: ");
    Console.WriteLine(badJson);
    var dto2 = JsonConvert.DeserializeObject<ModelBaseCollectionDTO>(badJson.ToString());
    Assert.IsTrue(false, "should not come here");
}
catch (JsonException ex)
{
    Assert.IsTrue(!new FileInfo("rce-test.txt").IsReadOnly);
    Console.WriteLine("Caught expected {0}: {1}", ex.GetType(), ex.Message);
}

Then the file rce-test.txt is not marked as read-only, and instead the following exception is thrown:

Newtonsoft.Json.JsonSerializationException: Type specified in JSON 'System.IO.FileInfo, mscorlib' is not compatible with 'Namespace.Models.ModelBase, Tile'. Path 'Models[0].$type', line 4, position 112.

Indicating that the attack gadget FileInfo is never even constructed.

Notes:

  • By using TypeNameHandling.Auto you avoid bloating your JSON with type information for non-polymorphic properties.

  • The correct format for the JSON in your post body can be determined by test-serializing the expected resulting ModelBaseCollectionDTO during development.

  • For an explanation of why the attack fails see External json vulnerable because of Json.Net TypeNameHandling auto?. As long as no attack gadget is compatible with (assignable to) your base model type, you should be secure.

  • Because you do not set TypeNameHandling in startup you are not making your other APIs vulnerable to attack or exposing yourself to attacks via deeply nested polymorphic properties such as those mentioned here.

  • Nevertheless, for added safety, you may still want to create a custom ISerializationBinder.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.