CVE-2011-0991: Resource Management Errors (CWE-399)
value: Use-after-free vulnerability in Mono, when Moonlight 2.x before 2.4.1 or 3.x before 3.99.3 is used, allows remote attackers to cause a denial of service or possibly have unspecified other impact via vectors related to finalizing and then resurrecting a DynamicMethod instance.
CWE description: Weaknesses in this category are related to improper management of system resources.
Published: 2011-04-13T21:55Z
Repository: https://github.com/mono/mono
Language: C
Files changed: mcs/class/corlib/System.Reflection.Emit/DynamicMethod.cs, mono/metadata/icall-def.h, mono/metadata/object-internals.h, mono/metadata/reflection.c, mono/metadata/runtime.c
Commit date: 2011-02-01 16:52:43 +0100
Commit message: Don't use finalization to cleanup dynamic methods.

	* reflection.c: Use a reference queue to cleanup
	dynamic methods instead of finalization.

	* runtime.c: Shutdown the dynamic method queue
	before runtime cleanup begins.

	* DynamicMethod.cs: No longer finalizable.

	* icall-def.h: Remove unused dynamic method icall.

	Fixes #660422

Diff: diff --git a/mcs/class/corlib/System.Reflection.Emit/DynamicMethod.cs b/mcs/class/corlib/System.Reflection.Emit/DynamicMethod.cs
index 9af56018aa1..6fb1600be4d 100644
--- a/mcs/class/corlib/System.Reflection.Emit/DynamicMethod.cs
+++ b/mcs/class/corlib/System.Reflection.Emit/DynamicMethod.cs
@@ -125,13 +125,10 @@ public DynamicMethod (string name, Type returnType, Type[] parameterTypes, bool
 		}
 
 		[MethodImplAttribute(MethodImplOptions.InternalCall)]
 		private extern void create_dynamic_method (DynamicMethod m);
 
-		[MethodImplAttribute(MethodImplOptions.InternalCall)]
-		private extern void destroy_dynamic_method (DynamicMethod m);
-
 		private void CreateDynMethod () {
 			if (mhandle.Value == IntPtr.Zero) {
 				if (ilgen == null || ilgen.ILOffset == 0)
 					throw new InvalidOperationException ("Method '" + name + "' does not have a method body.");
 
@@ -156,15 +153,10 @@ public DynamicMethod (string name, Type returnType, Type[] parameterTypes, bool
 
 				create_dynamic_method (this);
 			}
 		}
 
-		~DynamicMethod ()
-		{
-			destroy_dynamic_method (this);
-		}
-
 		[ComVisible (true)]
 		public Delegate CreateDelegate (Type delegateType)
 		{
 			if (delegateType == null)
 				throw new ArgumentNullException ("delegateType");
diff --git a/mono/metadata/icall-def.h b/mono/metadata/icall-def.h
index 26614eb4a79..9b894cbc202 100644
--- a/mono/metadata/icall-def.h
+++ b/mono/metadata/icall-def.h
@@ -522,11 +522,10 @@ ICALL_TYPE(DERIVEDTYPE, "System.Reflection.Emit.DerivedType", DERIVEDTYPE_1)
 ICALL(DERIVEDTYPE_1, "create_unmanaged_type", mono_reflection_create_unmanaged_type)
 #endif
 
 ICALL_TYPE(DYNM, "System.Reflection.Emit.DynamicMethod", DYNM_1)
 ICALL(DYNM_1, "create_dynamic_method", mono_reflection_create_dynamic_method)
-ICALL(DYNM_2, "destroy_dynamic_method", mono_reflection_destroy_dynamic_method)
 
 ICALL_TYPE(ENUMB, "System.Reflection.Emit.EnumBuilder", ENUMB_1)
 ICALL(ENUMB_1, "setup_enum_type", ves_icall_EnumBuilder_setup_enum_type)
 
 ICALL_TYPE(GPARB, "System.Reflection.Emit.GenericTypeParameterBuilder", GPARB_1)
diff --git a/mono/metadata/object-internals.h b/mono/metadata/object-internals.h
index 79280485bf5..345c6a810dc 100644
--- a/mono/metadata/object-internals.h
+++ b/mono/metadata/object-internals.h
@@ -1537,8 +1537,11 @@ MonoObject *
 mono_object_new_pinned (MonoDomain *domain, MonoClass *klass) MONO_INTERNAL;
 
 void
 mono_field_static_get_value_for_thread (MonoInternalThread *thread, MonoVTable *vt, MonoClassField *field, void *value) MONO_INTERNAL;
 
+void
+mono_reflection_shutdown (void) MONO_INTERNAL;
+
 #endif /* __MONO_OBJECT_INTERNALS_H__ */
 
 
diff --git a/mono/metadata/reflection.c b/mono/metadata/reflection.c
index e267172ccd9..d0252ab623a 100644
--- a/mono/metadata/reflection.c
+++ b/mono/metadata/reflection.c
@@ -11439,19 +11439,60 @@ mono_reflection_sighelper_get_signature_field (MonoReflectionSigHelper *sig)
 	sigbuffer_free (&buf);
 
 	return result;
 }
 
+typedef struct {
+	MonoMethod *handle;
+	MonoDomain *domain;
+} DynamicMethodReleaseData;
+	
+static MonoReferenceQueue *dynamic_method_queue;
+
+void
+mono_reflection_shutdown (void)
+{
+	MonoReferenceQueue *queue;
+	mono_loader_lock ();
+	queue = dynamic_method_queue;
+	dynamic_method_queue = NULL;
+	if (queue)
+		mono_gc_reference_queue_free (queue);
+	mono_loader_unlock ();
+}
+
+static void
+free_dynamic_method (void *dynamic_method)
+{
+	DynamicMethodReleaseData *data = dynamic_method;
+
+	mono_runtime_free_method (data->domain, data->handle);
+	g_free (data);
+}
+
 void 
 mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
 {
+	MonoReferenceQueue *queue;
+	MonoMethod *handle;
+	DynamicMethodReleaseData *release_data;
 	ReflectionMethodBuilder rmb;
 	MonoMethodSignature *sig;
 	MonoClass *klass;
 	GSList *l;
 	int i;
 
+	if (mono_runtime_is_shutting_down ())
+		mono_raise_exception (mono_get_exception_invalid_operation (""));
+
+	if (!(queue = dynamic_method_queue)) {
+		mono_loader_lock ();
+		if (!(queue = dynamic_method_queue))
+			queue = dynamic_method_queue = mono_gc_reference_queue_new (free_dynamic_method);
+		mono_loader_unlock ();
+	}
+
 	sig = dynamic_method_to_signature (mb);
 
 	reflection_methodbuilder_from_dynamic_method (&rmb, mb);
 
 	/*
@@ -11505,11 +11546,16 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
 		rmb.refs [i + 1] = handle_class;
 	}		
 
 	klass = mb->owner ? mono_class_from_mono_type (mono_reflection_type_get_handle ((MonoReflectionType*)mb->owner)) : mono_defaults.object_class;
 
-	mb->mhandle = reflection_methodbuilder_to_mono_method (klass, &rmb, sig);
+	mb->mhandle = handle = reflection_methodbuilder_to_mono_method (klass, &rmb, sig);
+	release_data = g_new (DynamicMethodReleaseData, 1);
+	release_data->handle = handle;
+	release_data->domain = mono_object_get_domain ((MonoObject*)mb);
+	if (!mono_gc_reference_queue_add (queue, (MonoObject*)mb, release_data))
+		g_free (release_data);
 
 	/* Fix up refs entries pointing at us */
 	for (l = mb->referenced_by; l; l = l->next) {
 		MonoReflectionDynamicMethod *method = (MonoReflectionDynamicMethod*)l->data;
 		MonoMethodWrapper *wrapper = (MonoMethodWrapper*)method->mhandle;
@@ -11531,20 +11577,10 @@ mono_reflection_create_dynamic_method (MonoReflectionDynamicMethod *mb)
 	mb->ilgen = NULL;
 }
 
 #endif /* DISABLE_REFLECTION_EMIT */
 
-void
-mono_reflection_destroy_dynamic_method (MonoReflectionDynamicMethod *mb)
-{
-	g_assert (mb);
-
-	if (mb->mhandle)
-		mono_runtime_free_method (
-			mono_object_get_domain ((MonoObject*)mb), mb->mhandle);
-}
-
 /**
  * 
  * mono_reflection_is_valid_dynamic_token:
  * 
  * Returns TRUE if token is valid.
diff --git a/mono/metadata/runtime.c b/mono/metadata/runtime.c
index e4a4c32b3a3..1a20d496d3e 100644
--- a/mono/metadata/runtime.c
+++ b/mono/metadata/runtime.c
@@ -37,7 +37,10 @@ fire_process_exit_event (MonoDomain *domain, gpointer user_data)
 
 void
 mono_runtime_shutdown (void)
 {
 	mono_domain_foreach (fire_process_exit_event, NULL);
+
+	/*From this point on, no more DM methods can be created. */
+	mono_reflection_shutdown ();
 }
 

Vulnerable code: 		[MethodImplAttribute(MethodImplOptions.InternalCall)]
		private extern void destroy_dynamic_method (DynamicMethod m);

		~DynamicMethod ()
		{
			destroy_dynamic_method (this);
		}

ICALL(DYNM_2, "destroy_dynamic_method", mono_reflection_destroy_dynamic_method)
	mb->mhandle = reflection_methodbuilder_to_mono_method (klass, &rmb, sig);
void
mono_reflection_destroy_dynamic_method (MonoReflectionDynamicMethod *mb)
{
	g_assert (mb);

	if (mb->mhandle)
		mono_runtime_free_method (
			mono_object_get_domain ((MonoObject*)mb), mb->mhandle);
}

Fixed code: void
mono_reflection_shutdown (void) MONO_INTERNAL;

typedef struct {
	MonoMethod *handle;
	MonoDomain *domain;
} DynamicMethodReleaseData;
	
static MonoReferenceQueue *dynamic_method_queue;

void
mono_reflection_shutdown (void)
{
	MonoReferenceQueue *queue;
	mono_loader_lock ();
	queue = dynamic_method_queue;
	dynamic_method_queue = NULL;
	if (queue)
		mono_gc_reference_queue_free (queue);
	mono_loader_unlock ();
}

static void
free_dynamic_method (void *dynamic_method)
{
	DynamicMethodReleaseData *data = dynamic_method;

	mono_runtime_free_method (data->domain, data->handle);
	g_free (data);
}

	MonoReferenceQueue *queue;
	MonoMethod *handle;
	DynamicMethodReleaseData *release_data;
	if (mono_runtime_is_shutting_down ())
		mono_raise_exception (mono_get_exception_invalid_operation (""));

	if (!(queue = dynamic_method_queue)) {
		mono_loader_lock ();
		if (!(queue = dynamic_method_queue))
			queue = dynamic_method_queue = mono_gc_reference_queue_new (free_dynamic_method);
		mono_loader_unlock ();
	}

	mb->mhandle = handle = reflection_methodbuilder_to_mono_method (klass, &rmb, sig);
	release_data = g_new (DynamicMethodReleaseData, 1);
	release_data->handle = handle;
	release_data->domain = mono_object_get_domain ((MonoObject*)mb);
	if (!mono_gc_reference_queue_add (queue, (MonoObject*)mb, release_data))
		g_free (release_data);

	/*From this point on, no more DM methods can be created. */
	mono_reflection_shutdown ();