0

I use a package that contains code for a tcp server and client that is really easy to use, problem is it uses Newtonsoft Json serialization and deserialization with TypeNameHandling.All to send an receive messages, and client or servers could be untrusted sources.

public static JsonSerializerSettings JsonSettings = new()
        {
            TypeNameHandling = TypeNameHandling.All,

        };
[...]
[...]
return JsonConvert.DeserializeObject<INetMessage>(message.Substring(8), JsonSettings);

It also contains such a class:

public class NetMessage<T> : INetMessage where T: ISafeNetSerialization
    {
        public ulong snowflake { get; set; }

        public T Content { get; set; }

        public NetMessage(T content)
        {
            this.Content = content;
        }

        public override string ToString()
        {
            return Content.ToString();
        }
    }

Can I use a code snippet using reflection, to go through all types inheriting INetMessage and ISafeNetSerialization interfaces and check if they can possibly contain something (or contain something that contains... etc) like an object, a dynamic or an Exception, a CollectionBase and other untyped objects and collections, including any Generic types inheriting those two that could be added in another library ?

I know that I should especially look for TempFileCollection and ObjectDataProvider.

Code snippet would then be used inside an unit test or at runtime before the initialization of the first server / and / or tcp client.

holeo hlw
  • 13
  • 4
  • Before you serialize, validate that the string doesnt contain a substring that might corrupt the handling of the code. However, I would say, I don't see how a serialization would be able to cause your system to do something unwanted? If you simply attempt to serialize the data in a try catch, you will either A, get an object or B, not. Or do you asusume there is a secuirty flaw in newtonsoft? I don't think that would be a thing, rather it would have to be something the coder did, in conjuction with newtonsofts deserializer. – Morten Bork Jan 11 '22 at 15:56
  • https://stackoverflow.com/a/49040862/11069905 contains an explaination of how TypeNameHandling could cause a vulnerability, by the deserialization of an object containing potentially malicious code inside of its constructor, disposer, or population of the properties. – holeo hlw Jan 11 '22 at 16:01
  • The answer is at the bottom: "Thank you very much for your detailed answer! So the solution is to use a SerializationBinder and "Whitelist" all possible allowed types. – Anika Leser" Mar 1 '18 at 10:14 I didn't know that this was an issue! thanks for the heads up – Morten Bork Jan 11 '22 at 16:07
  • Whitelisting trusted types is really the only reliable solution for this. There is literally no limit to the amount of cleverness arbitrary code could get up to to sneak in undesirable constructs, and any solution trying to specifically blacklist those is never going to be reliable. Failing that, isolating code to run on sandboxed VMs where it essentially can do no harm is another approach, but that has its own difficulties, since eventually it will have to interact with other systems somewhere. – Jeroen Mostert Jan 11 '22 at 16:08
  • Problem is, Whitelisting trusted types is tricky. I just learnt that, for example, Exceptions are risky to use. It can be real hard and long to check to know if the type i believe to be trusted doesn't contain one dynamic type somewhere in its code. The "ISafeNetSerialization" interface is exactly that, an empty interface that is just used to mark inherited class as trusted, but a second check (basically to prevent someone using the library without knowing the risks from doing the mistake of putting a bad type and mark it as trusted) would be nice. – holeo hlw Jan 11 '22 at 16:21
  • Even if you don't have to guard against active malice, attempting to identify every possible way types could be referred to dynamically is, well, challenging. A whitelist can be made to work if it's implemented as early as possible -- that is, don't decompile code and attempt to see if it's OK, but make sure that when someone is writing a type they can't include constructs that aren't allowed. This would require some form of static analysis (like a custom Roslyn analyzer). Quite a bit of work, but still far less than trying to reflect over compiled code. – Jeroen Mostert Jan 11 '22 at 16:27

0 Answers0