1
Vote

JsonTypeReflector.DynamicCodeGeneration Security Issue

description

Hello,
 
I think there is an issue with the way JsonTypeReflector.DynamicCodeGeneration is set. It is a lazy static getter that is never updated. The value of JsonTypeReflector.DynamicCodeGeneration depends on security level access:
 
new ReflectionPermission(ReflectionPermissionFlag.MemberAccess).Demand();
new ReflectionPermission(ReflectionPermissionFlag.RestrictedMemberAccess).Demand();
new SecurityPermission(SecurityPermissionFlag.UnmanagedCode).Demand();
 
If a unrestricted AppDomain first deserialized an object, JsonTypeReflector.DynamicCodeGeneration will be set to True. Then if we deserialize an object through another AppDomain that is restricted, DynamicCodeGeneration is not updated, so the DynamicReflectionDelegateFactory.CreateDynamicMethod will throw an exception.
 
My fix is to re initialize JsonTypeReflector.DynamicCodeGeneration before each deserialization, because you can never know what permission you have statically and for all the life of the app domain.

comments

B4ptiste wrote Sep 27, 2011 at 5:18 PM

another fix would be to allow to reset JsonTypeReflector.DynamicCodeGeneration

JamesNK wrote Oct 1, 2011 at 11:23 AM

I thought static member's were scoped per AppDomain?

B4ptiste wrote Oct 1, 2011 at 4:23 PM

They are scoped by AppDomain. But the problem I encounter is that if you first run a deserialization in AppDomain 1. Then you have some code which runs in AppDomain 2 (with restricted rights) and have a proxy to an object that lives in AppDomain 1. When it calls deserialization in AppDomain1 from AppDomain2, you don't have the enough rights anymore.

I don't know if I make myself clear. But I just want to say that setting only one time JsonTypeReflector.DynamicCodeGeneration, based on permission is wrong, because this permissions can change based on the stack of AppDomain

JamesNK wrote Oct 1, 2011 at 10:25 PM

Ok. For performance I don't want it tested every time, but what I could do is make it manually settable, that way you can either update it yourself when you know permissions have changed or wrap calls to the serializer in the test yourself so it happens every time. Is that good with you?

B4ptiste wrote Oct 2, 2011 at 2:12 PM

No problem ! this is exactly the spirit of the patch I sent. I added a setter in JsonTypeReflector.DynamicCodeGeneration and because JsonTypeReflector is internal. I add also a setter and getter in JsonSerializerSetting

B4ptiste wrote Oct 2, 2011 at 2:19 PM

Patch id is 10474

B4ptiste wrote Oct 3, 2011 at 7:16 AM

ho, I just saw that you did the release 3 of your library... it is too bad that you didn't find the time to include my very small patch in it. I will have to patch again your code.

Sign in to add a comment or to set email notifications