[Mono]: Fix static closed delegate fnptr crash. (#68701)

* Fix static closed delegate fnptr crash.

When accessing a function pointer to a static closed delegate like
done in added test:

GetFunctionPointerForDelegate_MarshalledClosedStaticDelegate

it will trigger a read outside of the allocated mspecs buffer since
invoke_sig and method signature arguments wont match.

There was already logic to adjust this in emit_managed_wrapper_ilgen,
but done after call to emit_managed_wrapper_validate_signature
that will touch memory and depending on its content, trigger a crash.

Fix make sure we do signature adjustments first and then validate
the signature. Fix also adjust a couple of mspecs allocations to use
g_new0 as all others to make sure we get NULL pointers in the mspecs
array.

Since this scenario was not covered on CI, commit also adds a new test
in GetFunctionPointerForDelegateTests.cs covering this scenario.

Author: lateralusX

src/libraries/System.Runtime.InteropServices/tests/System.Runtime.InteropServices.UnitTests/System/Runtime/InteropServices/Marshal/GetFunctionPointerForDelegateTests.cs

@@ -92,8 +92,29 @@ public void GetFunctionPointer_GenericDelegate_ThrowsArgumentException()
             AssertExtensions.Throws<ArgumentException>("delegate", () => Marshal.GetFunctionPointerForDelegate(d));
         }
 
+        [Fact]
+        [SkipOnPlatform(TestPlatforms.Browser, "Not supported on Browser.")]
+        public void GetFunctionPointerForDelegate_MarshalledOpenStaticDelegate()
+        {
+            MethodInfo targetMethod = typeof(GetFunctionPointerForDelegateTests).GetMethod(nameof(Method), BindingFlags.NonPublic | BindingFlags.Static);
+            Delegate original = targetMethod.CreateDelegate(typeof(NonGenericDelegate), null);
+            IntPtr ptr = Marshal.GetFunctionPointerForDelegate(original);
+            Assert.NotEqual(IntPtr.Zero, ptr);
+        }
+
+        [Fact]
+        [SkipOnPlatform(TestPlatforms.Browser, "Not supported on Browser.")]
+        public void GetFunctionPointerForDelegate_MarshalledClosedStaticDelegate()
+        {
+            MethodInfo targetMethod = typeof(GetFunctionPointerForDelegateTests).GetMethod(nameof(Method), BindingFlags.NonPublic | BindingFlags.Static);
+            Delegate original = targetMethod.CreateDelegate(typeof(NoArgsDelegate), "value");
+            IntPtr ptr = Marshal.GetFunctionPointerForDelegate(original);
+            Assert.NotEqual(IntPtr.Zero, ptr);
+        }
+
         public delegate void GenericDelegate<T>(T t);
         public delegate void NonGenericDelegate(string t);
+        public delegate void NoArgsDelegate();
 
         private static void Method(string s) { }
     }

src/mono/mono/metadata/cominterop.c

@@ -2089,7 +2089,7 @@ cominterop_get_ccw_method (MonoClass *iface, MonoMethod *method, MonoError *erro
 	adjust_method = cominterop_get_managed_wrapper_adjusted (method);
 	sig_adjusted = mono_method_signature_internal (adjust_method);
 
-	mspecs = g_new (MonoMarshalSpec*, sig_adjusted->param_count + 1);
+	mspecs = g_new0 (MonoMarshalSpec*, sig_adjusted->param_count + 1);
 	mono_method_get_marshal_info (method, mspecs);
 
 	/* move managed args up one */

src/mono/mono/metadata/marshal-ilgen.c

@@ -743,7 +743,7 @@ emit_native_wrapper_validate_signature (MonoMethodBuilder *mb, MonoMethodSignatu
 	if (mspecs) {
 		for (int i = 0; i < sig->param_count; i ++) {
 			if (mspecs [i + 1] && mspecs [i + 1]->native == MONO_NATIVE_CUSTOM) {
-				if (!mspecs [i + 1]->data.custom_data.custom_name || strlen (mspecs [i + 1]->data.custom_data.custom_name) == 0) {
+				if (!mspecs [i + 1]->data.custom_data.custom_name || *mspecs [i + 1]->data.custom_data.custom_name == '\0') {
 					mono_mb_emit_exception_full (mb, "System", "TypeLoadException", g_strdup ("Missing ICustomMarshaler type"));
 					return FALSE;
 				}
@@ -5023,7 +5023,7 @@ emit_managed_wrapper_validate_signature (MonoMethodSignature* sig, MonoMarshalSp
 	if (mspecs) {
 		for (int i = 0; i < sig->param_count; i ++) {
 			if (mspecs [i + 1] && mspecs [i + 1]->native == MONO_NATIVE_CUSTOM) {
-				if (!mspecs [i + 1]->data.custom_data.custom_name || strlen (mspecs [i + 1]->data.custom_data.custom_name) == 0) {
+				if (!mspecs [i + 1]->data.custom_data.custom_name || *mspecs [i + 1]->data.custom_data.custom_name == '\0') {
 					mono_error_set_generic_error (error, "System", "TypeLoadException", "Missing ICustomMarshaler type");
 					return FALSE;
 				}
@@ -5067,8 +5067,21 @@ emit_managed_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *invoke_s
 	sig = m->sig;
 	csig = m->csig;
 
-	if (!emit_managed_wrapper_validate_signature (sig, mspecs, error))
+	if (!sig->hasthis && sig->param_count != invoke_sig->param_count) {
+		/* Closed delegate */
+		g_assert (sig->param_count == invoke_sig->param_count + 1);
+		closed = TRUE;
+		/* Use a new signature without the first argument */
+		sig = mono_metadata_signature_dup (sig);
+		memmove (&sig->params [0], &sig->params [1], (sig->param_count - 1) * sizeof (MonoType*));
+		sig->param_count --;
+	}
+
+	if (!emit_managed_wrapper_validate_signature (sig, mspecs, error)) {
+		if (closed)
+			g_free (sig);
 		return;
+	}
 
 	MonoType *int_type = mono_get_int_type ();
 	MonoType *boolean_type = m_class_get_byval_arg (mono_defaults.boolean_class);
@@ -5079,16 +5092,6 @@ emit_managed_wrapper_ilgen (MonoMethodBuilder *mb, MonoMethodSignature *invoke_s
 	/* allocate local 2 (boolean) delete_old */
 	mono_mb_add_local (mb, boolean_type);
 
-	if (!sig->hasthis && sig->param_count != invoke_sig->param_count) {
-		/* Closed delegate */
-		g_assert (sig->param_count == invoke_sig->param_count + 1);
-		closed = TRUE;
-		/* Use a new signature without the first argument */
-		sig = mono_metadata_signature_dup (sig);
-		memmove (&sig->params [0], &sig->params [1], (sig->param_count - 1) * sizeof (MonoType*));
-		sig->param_count --;
-	}
-
 	if (!MONO_TYPE_IS_VOID(sig->ret)) {
 		/* allocate local 3 to store the return value */
 		mono_mb_add_local (mb, sig->ret);

src/mono/mono/metadata/marshal.c

@@ -3561,7 +3561,7 @@ mono_marshal_get_native_wrapper (MonoMethod *method, gboolean check_exceptions,
 	csig = mono_metadata_signature_dup_full (get_method_image (method), sig);
 	mono_marshal_set_callconv_from_modopt (method, csig, FALSE);
 
-	mspecs = g_new (MonoMarshalSpec*, sig->param_count + 1);
+	mspecs = g_new0 (MonoMarshalSpec*, sig->param_count + 1);
 	mono_method_get_marshal_info (method, mspecs);
 
 	if (mono_class_try_get_suppress_gc_transition_attribute_class ()) {