Type validation holes when using Reflection to invoke a method with a void* parameter.
Closed this issue · 3 comments
Consider the following test app:
namespace ConsoleApp1
{
public unsafe class GG
{
public static void Main()
{
Test(UIntPtr.Zero);
}
public static void Test(object parameterToPassIn)
{
MyBinder binder = new MyBinder();
MethodInfo m = typeof(GG).GetMethod("Foo");
object[] args = { parameterToPassIn };
m.Invoke(null, BindingFlags.Default, binder, args, null);
if (!binder.ChangeTypeCalled)
{
Console.WriteLine("Failed. Binder not given chance to fix things.");
return;
}
Console.WriteLine("Passed.");
return;
}
public static void Foo(void*pv)
{
}
}
public sealed unsafe class MyBinder : Binder
{
public bool ChangeTypeCalled { get; private set; }
public sealed override FieldInfo BindToField(BindingFlags bindingAttr, FieldInfo[] match, object value, CultureInfo culture)
{
throw new NotImplementedException();
}
public sealed override MethodBase BindToMethod(BindingFlags bindingAttr, MethodBase[] match, ref object[] args, ParameterModifier[] modifiers, CultureInfo culture, string[] names, out object state)
{
throw new NotImplementedException();
}
public sealed override object ChangeType(object value, Type type, CultureInfo culture)
{
ChangeTypeCalled = true;
return Pointer.Box((void*)0, typeof(void*));
}
public sealed override void ReorderArgumentArray(ref object[] args, object state)
{
throw new NotImplementedException();
}
public sealed override MethodBase SelectMethod(BindingFlags bindingAttr, MethodBase[] match, Type[] types, ParameterModifier[] modifiers)
{
throw new NotImplementedException();
}
public sealed override PropertyInfo SelectProperty(BindingFlags bindingAttr, PropertyInfo[] match, Type returnType, Type[] indexes, ParameterModifier[] modifiers)
{
throw new NotImplementedException();
}
}
}
Passing a UIntPtr to a void* isn't allowed so what should happen is that Reflection should call MyBinder.ChangeType() to give it a chance to convert the UIntPtr into something that is acceptable.
Instead, Reflection throws an ArgumentException with the message "Object type cannot be converted to target type." and never calls the binder.
That is, on release builds.
On checked builds, it triggers a GC_NOTRIGGER contract violation while propagating the exception:
GC_TRIGGERS encountered in a GC_NOTRIGGER scope
CONTRACT in CLRException::GetThrowableFromException at "c:\dd\coreclr\src\vm\clrex.cpp" @ 714
GCX_COOP in UnwindAndContinueRethrowHelperInsideCatch at "c:\dd\coreclr\src\vm\excep.cpp" @ 8566
CONTRACT in MethodDescCallSite::CallTargetWorker at "c:\dd\coreclr\src\vm\callhelpers.cpp" @ 361
GCX_COOP in Assembly::ExecuteMainMethod at "c:\dd\coreclr\src\vm\assembly.cpp" @ 2468
CONTRACT in Assembly::ExecuteMainMethod at "c:\dd\coreclr\src\vm\assembly.cpp" @ 2452
GCX_COOP in CorHost2::ExecuteAssembly at "c:\dd\coreclr\src\vm\corhost.cpp" @ 651
CONTRACT in CorHost2::ExecuteAssembly at "c:\dd\coreclr\src\vm\corhost.cpp" @ 595
CORECLR! CONTRACT_ASSERT + 0x342 (0x00007ffa`f693acf2)
CORECLR! EEContract::DoChecks + 0x3DE (0x00007ffa`f6ad1b8e)
CORECLR! CLRException::GetThrowableFromException + 0x14B (0x00007ffa`f6b164cb)
CORECLR! UnwindAndContinueRethrowHelperInsideCatch + 0xBB (0x00007ffa`f6acf6fb)
CORECLR! `RuntimeMethodHandle::InvokeMethod'::`1'::catch$11 + 0xF4 (0x00007ffa`f78c308f)
CORECLR! CallSettingFrame + 0x20 (0x00007ffa`f7611990)
CORECLR! _CxxCallCatchBlock + 0x15A (0x00007ffa`f760c16a)
NTDLL! RtlCaptureContext + 0x3C3 (0x00007ffb`442ea193)
CORECLR! RuntimeMethodHandle::InvokeMethod + 0xDEE (0x00007ffa`f72e246e)
The problems are multifold. First, when the formal parameter type is a pointer to void, the code that's supposed to catch invalid arguments and divert to the binder (https://github.com/dotnet/coreclr/blob/master/src/vm/reflectioninvocation.cpp#L298) abdicates from the duty and declares virtually any input argument as legal. So the binder never gets called and Reflection proceeds to execute the call.
However, the calling mechanism performs its own type checks so which happens at InvokeUtil::CopyArg() (https://github.com/dotnet/coreclr/blob/master/src/vm/invokeutil.cpp#L299)
case ELEMENT_TYPE_PTR:
case ELEMENT_TYPE_FNPTR:
{
// If we got the univeral zero...Then assign it and exit.
if (rObj == 0) {
*(PVOID *)pArgDst = 0;
}
else {
if (rObj->GetMethodTable() == MscorlibBinder::GetClassIfExist(CLASS__POINTER) && type == ELEMENT_TYPE_PTR)
*(PVOID *)pArgDst = GetPointerValue(rObj);
else if (rObj->GetTypeHandle().AsMethodTable() == MscorlibBinder::GetElementType(ELEMENT_TYPE_I))
{
ARG_SLOT slot;
CreatePrimitiveValue(oType, oType, rObj, &slot);
*(PVOID *)pArgDst = (PVOID)slot;
}
else
COMPlusThrow(kArgumentException,W("Arg_ObjObj"));
}
break;
}
Firstly, there's the unguarded call to "AsMethodTable()" for the purpose of seeing if "rObj" is a System.IntPtr. If you pass in something like an int[], this triggers an !IsTypeDesc() assertion inside AsMethodTable. Since the returned "pointer" value is only compared to another constant pointer value, there's no major risk here but it is shabby.
Two, the COMPlusThrow() at this point triggers the aforementioned GC_NOTRIGGER violation while bubbling up.
Managed call stack:
System.ArgumentException
HResult=0x80070057
Message=Object type cannot be converted to target type.
Source=mscorlib
StackTrace:
at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor)
at System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments)
at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
at ConsoleApp1.GG.Test(Object parameterToPassIn) in C:\Users\sharter.NORTHAMERICA\source\repos\ConsoleApp17\Program.cs:line 19
at ConsoleApp1.GG.Main() in Program.cs:line 11
Moving to future based on schedule + priority.
Please add additional hits to this issue to increase priority.
Passing a UIntPtr to a void* isn't allowed so what should happen is that Reflection should call MyBinder.ChangeType() to give it a chance to convert the UIntPtr into something that is acceptable.
There is explicit operators for UIntPtr to void * and vise versa https://docs.microsoft.com/en-us/dotnet/api/system.uintptr.op_explicit?view=net-6.0#system-uintptr-op-explicit(system-void*)-system-uintptr, so I think it should just work similarly how it works for IntPtr