Bladeren bron

Prevent GetType and System.Reflection namespace under interop by default (#994)

Marko Lahma 3 jaren geleden
bovenliggende
commit
c117c5ea16

+ 2 - 0
Jint.Tests/Runtime/Domain/HiddenMembers.cs

@@ -18,5 +18,7 @@ namespace Jint.Tests.Runtime.Domain
         public string Method1() => "Method1";
 
         public string Method2() => "Method2";
+
+        public Type Type => GetType();
     }
 }

+ 23 - 18
Jint.Tests/Runtime/InteropTests.MemberAccess.cs

@@ -70,33 +70,38 @@ namespace Jint.Tests.Runtime
             Assert.True(engine.Evaluate("m.Field2").IsString());
             Assert.True(engine.Evaluate("m.Member2").IsString());
             Assert.True(engine.Evaluate("m.Method2()").IsString());
+
+            // we forbid GetType by default
+            Assert.True(engine.Evaluate("m.GetType").IsUndefined());
         }
 
         [Fact]
-        public void ShouldBeAbleToHideGetType()
+        public void ShouldBeAbleToExposeGetType()
         {
-            var engine = new Engine(options => options
-                .SetTypeResolver(new TypeResolver
-                {
-                    MemberFilter = member => !Attribute.IsDefined(member, typeof(ObsoleteAttribute))
-                })
-            );
+            var engine = new Engine(options =>
+            {
+                options.Interop.AllowGetType = true;
+                options.Interop.AllowSystemReflection = true;
+            });
             engine.SetValue("m", new HiddenMembers());
-
-            Assert.True(engine.Evaluate("m.Method1").IsUndefined());
+            Assert.True(engine.Evaluate("m.GetType").IsCallable());
 
             // reflection could bypass some safeguards
             Assert.Equal("Method1", engine.Evaluate("m.GetType().GetMethod('Method1').Invoke(m, [])").AsString());
+        }
 
-            // but not when we forbid GetType
-            var hiddenGetTypeEngine = new Engine(options => options
-                .SetTypeResolver(new TypeResolver
-                {
-                    MemberFilter = member => member.Name != nameof(GetType)
-                })
-            );
-            hiddenGetTypeEngine.SetValue("m", new HiddenMembers());
-            Assert.True(hiddenGetTypeEngine.Evaluate("m.GetType").IsUndefined());
+        [Fact]
+        public void ShouldProtectFromReflectionServiceUsage()
+        {
+            var engine = new Engine();
+            engine.SetValue("m", new HiddenMembers());
+
+            // we can get a type reference if it's exposed via property, bypassing GetType
+            var type = engine.Evaluate("m.Type");
+            Assert.IsType<ObjectWrapper>(type);
+
+            var ex = Assert.Throws<InvalidOperationException>(() => engine.Evaluate("m.Type.Module.GetType().Module.GetType('System.DateTime')"));
+            Assert.Equal("Cannot access System.Reflection namespace, check Engine's interop options", ex.Message);
         }
 
         [Fact]

+ 1 - 1
Jint/Options.Extensions.cs

@@ -152,7 +152,7 @@ namespace Jint
 
         public static Options AllowOperatorOverloading(this Options options, bool allow = true)
         {
-            options.Interop.OperatorOverloadingAllowed = allow;
+            options.Interop.AllowOperatorOverloading = allow;
             return options;
         }
 

+ 13 - 1
Jint/Options.cs

@@ -187,6 +187,18 @@ namespace Jint
         /// </summary>
         public bool Enabled { get; set; }
 
+        /// <summary>
+        /// Whether to expose <see cref="object.GetType"></see> which can allow bypassing allow lists and open a way to reflection.
+        /// Defaults to false.
+        /// </summary>
+        public bool AllowGetType { get; set; }
+
+        /// <summary>
+        /// Whether Jint should allow wrapping objects from System.Reflection namespace.
+        /// Defaults to false.
+        /// </summary>
+        public bool AllowSystemReflection { get; set; }
+
         /// <summary>
         /// Whether writing to CLR objects is allowed (set properties), defaults to true.
         /// </summary>
@@ -195,7 +207,7 @@ namespace Jint
         /// <summary>
         /// Whether operator overloading resolution is allowed, defaults to false.
         /// </summary>
-        public bool OperatorOverloadingAllowed { get; set; }
+        public bool AllowOperatorOverloading { get; set; }
 
         /// <summary>
         /// Types holding extension methods that should be considered when resolving methods.

+ 8 - 0
Jint/Runtime/Interop/DefaultObjectConverter.cs

@@ -67,6 +67,14 @@ namespace Jint
                     else
                     {
                         var t = value.GetType();
+
+                        if (!engine.Options.Interop.AllowSystemReflection
+                            && t.Namespace?.StartsWith("System.Reflection") == true)
+                        {
+                            const string message = "Cannot access System.Reflection namespace, check Engine's interop options";
+                            ExceptionHelper.ThrowInvalidOperationException(message);
+                        }
+
                         if (t.IsEnum)
                         {
                             var ut = Enum.GetUnderlyingType(t);

+ 1 - 1
Jint/Runtime/Interop/DefaultTypeConverter.cs

@@ -212,7 +212,7 @@ namespace Jint.Runtime.Interop
                 return obj;
             }
 
-            if (_engine.Options.Interop.OperatorOverloadingAllowed)
+            if (_engine.Options.Interop.AllowOperatorOverloading)
             {
 #if NETSTANDARD
                 var key = (valueType, type);

+ 1 - 1
Jint/Runtime/Interop/Reflection/IndexerAccessor.cs

@@ -74,7 +74,7 @@ namespace Jint.Runtime.Interop.Reflection
                 return null;
             }
 
-            var filter = engine.Options.Interop.TypeResolver.MemberFilter;
+            var filter = new Func<MemberInfo, bool>(m => engine.Options.Interop.TypeResolver.Filter(engine, m));
 
             // default indexer wins
             if (typeof(IList).IsAssignableFrom(targetType) && filter(_iListIndexer))

+ 1 - 1
Jint/Runtime/Interop/TypeReference.cs

@@ -165,7 +165,7 @@ namespace Jint.Runtime.Interop
             }
 
             const BindingFlags bindingFlags = BindingFlags.Public | BindingFlags.Static;
-            return typeResolver.TryFindMemberAccessor(type, name, bindingFlags, indexerToTry: null, out var accessor)
+            return typeResolver.TryFindMemberAccessor(_engine, type, name, bindingFlags, indexerToTry: null, out var accessor)
                 ? accessor
                 : ConstantValueAccessor.NullAccessor;
         }

+ 16 - 8
Jint/Runtime/Interop/TypeResolver.cs

@@ -12,15 +12,22 @@ namespace Jint.Runtime.Interop
     /// </summary>
     public sealed class TypeResolver
     {
-        public static readonly TypeResolver Default = new TypeResolver();
+        public static readonly TypeResolver Default = new();
 
         private Dictionary<ClrPropertyDescriptorFactoriesKey, ReflectionAccessor> _reflectionAccessors = new();
 
         /// <summary>
         /// Registers a filter that determines whether given member is wrapped to interop or returned as undefined.
+        /// By default allows all but will also be limited by <see cref="InteropOptions.AllowGetType"/> configuration.
         /// </summary>
+        /// <seealso cref="InteropOptions.AllowGetType"/>
         public Predicate<MemberInfo> MemberFilter { get; set; } = _ => true;
 
+        internal bool Filter(Engine engine, MemberInfo m)
+        {
+            return (engine.Options.Interop.AllowGetType || m.Name != nameof(GetType)) && MemberFilter(m);
+        }
+
         /// <summary>
         /// Gives the exposed names for a member. Allows to expose C# convention following member like IsSelected
         /// as more JS idiomatic "selected" for example. Defaults to returning the <see cref="MemberInfo.Name"/> as-is.
@@ -73,7 +80,7 @@ namespace Jint.Runtime.Interop
             const BindingFlags bindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
 
             // properties and fields cannot be numbers
-            if (!isNumber && TryFindMemberAccessor(type, memberName, bindingFlags, indexer, out var temp))
+            if (!isNumber && TryFindMemberAccessor(engine, type, memberName, bindingFlags, indexer, out var temp))
             {
                 return temp;
             }
@@ -97,7 +104,7 @@ namespace Jint.Runtime.Interop
             {
                 foreach (var iprop in iface.GetProperties())
                 {
-                    if (!MemberFilter(iprop))
+                    if (!Filter(engine, iprop))
                     {
                         continue;
                     }
@@ -130,7 +137,7 @@ namespace Jint.Runtime.Interop
             {
                 foreach (var imethod in iface.GetMethods())
                 {
-                    if (!MemberFilter(imethod))
+                    if (!Filter(engine, imethod))
                     {
                         continue;
                     }
@@ -165,7 +172,7 @@ namespace Jint.Runtime.Interop
                 var matches = new List<MethodInfo>();
                 foreach (var method in extensionMethods)
                 {
-                    if (!MemberFilter(method))
+                    if (!Filter(engine, method))
                     {
                         continue;
                     }
@@ -189,6 +196,7 @@ namespace Jint.Runtime.Interop
         }
 
         internal bool TryFindMemberAccessor(
+            Engine engine,
             Type type,
             string memberName,
             BindingFlags bindingFlags,
@@ -201,7 +209,7 @@ namespace Jint.Runtime.Interop
             var typeResolverMemberNameCreator = MemberNameCreator;
             foreach (var p in type.GetProperties(bindingFlags))
             {
-                if (!MemberFilter(p))
+                if (!Filter(engine, p))
                 {
                     continue;
                 }
@@ -231,7 +239,7 @@ namespace Jint.Runtime.Interop
             FieldInfo field = null;
             foreach (var f in type.GetFields(bindingFlags))
             {
-                if (!MemberFilter(f))
+                if (!Filter(engine, f))
                 {
                     continue;
                 }
@@ -256,7 +264,7 @@ namespace Jint.Runtime.Interop
             List<MethodInfo> methods = null;
             foreach (var m in type.GetMethods(bindingFlags))
             {
-                if (!MemberFilter(m))
+                if (!Filter(engine, m))
                 {
                     continue;
                 }

+ 1 - 1
Jint/Runtime/Interpreter/EvaluationContext.cs

@@ -12,7 +12,7 @@ namespace Jint.Runtime.Interpreter
             Engine = engine;
             DebugMode = engine._isDebugMode;
             ResumedCompletion = resumedCompletion ?? default; // TODO later
-            OperatorOverloadingAllowed = engine.Options.Interop.OperatorOverloadingAllowed;
+            OperatorOverloadingAllowed = engine.Options.Interop.AllowOperatorOverloading;
         }
 
         public Engine Engine { get; }