Merge pull request #1331 from Unity-Technologies/unity-master-debugger-fix-1269030

[debugger] Access invalid memory address using PointerValue Command. …

mcs/class/Mono.Debugger.Soft/Mono.Debugger.Soft/PointerValue.cs

@@ -51,10 +51,19 @@ namespace Mono.Debugger.Soft
 		// Since protocol version 2.46
 		public Value Value {
 			get {
+				ValueImpl value;
 				if (Address == 0)
 					return null;
-
-				return vm.DecodeValue (vm.conn.Pointer_GetValue (Address, Type));
+				try {
+					value = vm.conn.Pointer_GetValue (Address, Type);
+				}
+				catch (CommandException ex) {
+				if (ex.ErrorCode == ErrorCode.INVALID_ARGUMENT)
+					throw new ArgumentException ("Invalid pointer address.");
+				else
+					throw;
+				}
+				return vm.DecodeValue (value);
 			}
 		}
 

mcs/class/Mono.Debugger.Soft/Test/dtest-app.cs

@@ -377,6 +377,11 @@ public class Tests : TestsBase, ITest2
 		test_async_debug_generics();
 		test_invalid_argument_assembly_get_type ();
 
+		if (args.Length >0 && args [0] == "pointer_arguments2") {
+			pointers2 ();
+			return 0;
+		}
+
 		return 3;
 	}
 
@@ -1787,6 +1792,17 @@ public class Tests : TestsBase, ITest2
 	public static ref BlittableStruct get_ref_struct () {
 		return ref ref_return_struct;
 	}
+
+	public static unsafe void pointer_arguments2 (int* a) {
+		*a = 0;
+	}
+
+	[MethodImplAttribute (MethodImplOptions.NoInlining)]
+	public static unsafe void pointers2 () {
+		int[] a = new [] {1,2,3};
+		fixed (int* pa = a)
+			pointer_arguments2 (pa);
+	}
 }
 
 public class SentinelClass : MarshalByRefObject {

mcs/class/Mono.Debugger.Soft/Test/dtest.cs

@@ -4470,5 +4470,30 @@ public class DebuggerTests
 		Value v = o.InvokeMethod (e.Thread, m, null, InvokeOptions.DisableBreakpoints);
 		AssertValue ("Foo", v);
 	}
+
+	[Test]
+	public void InvalidPointer_GetValue () {
+		vm.Detach ();
+		Start (dtest_app_path, "pointer_arguments2");
+		Event e = run_until ("pointer_arguments2");
+		var frame = e.Thread.GetFrames () [0];
+
+		var param = frame.Method.GetParameters()[0];
+		Assert.AreEqual("Int32*", param.ParameterType.Name);
+
+		var pointerValue = frame.GetValue(param) as PointerValue;
+		Assert.AreEqual("Int32*", pointerValue.Type.Name);
+
+
+		var pointerValue2 = new PointerValue (pointerValue.VirtualMachine, pointerValue.Type, 200);
+
+		try {
+			var val = pointerValue2.Value;
+		}
+		catch (ArgumentException ex) {
+			Assert.IsInstanceOfType (typeof (ArgumentException), ex);
+		}
+	}
+
 } // class DebuggerTests
 } // namespace

mono/mini/debugger-agent.c

@@ -45,6 +45,7 @@
 #include <process.h>
 #endif
 #include <ws2tcpip.h>
+#include <windows.h>
 #endif
 
 #ifdef HOST_ANDROID
@@ -108,6 +109,13 @@
 
 #include <mono/utils/mono-os-mutex.h>
 
+#include <fcntl.h>
+#include <sys/stat.h>
+
+#ifndef S_IWUSR
+	#define S_IWUSR S_IWRITE
+#endif
+
 #define THREAD_TO_INTERNAL(thread) (thread)->internal_thread
 
 #include "debugger-agent.h"
@@ -709,6 +717,10 @@ static MonoThreadHandle *debugger_thread_handle;
 
 static int log_level;
 
+static int file_check_valid_memory = -1;
+
+static char* filename_check_valid_memory;
+
 static gboolean embedding;
 
 static FILE *log_file;
@@ -1281,6 +1293,12 @@ mono_debugger_agent_cleanup (void)
 	breakpoints_cleanup ();
 	objrefs_cleanup ();
 	ids_cleanup ();
+
+	if (file_check_valid_memory != -1) {
+		remove (filename_check_valid_memory);
+		g_free (filename_check_valid_memory);
+		close (file_check_valid_memory);
+	}
 }
 
 /*
@@ -11711,6 +11729,46 @@ string_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 	return ERR_NONE;
 }
 
+#ifndef _MSC_VER
+static void 
+create_file_to_check_memory_address (void) 
+{
+	if (file_check_valid_memory != -1)
+		return;
+	char *file_name = g_strdup_printf ("debugger_check_valid_memory.%d", getpid());
+	filename_check_valid_memory = g_build_filename (g_get_tmp_dir (), file_name, (const char*)NULL);
+	file_check_valid_memory = open(filename_check_valid_memory, O_CREAT | O_WRONLY | O_APPEND, S_IWUSR);
+	g_free (file_name);
+}
+#endif
+
+static gboolean 
+valid_memory_address (gpointer addr, gint size)
+{
+#ifndef _MSC_VER
+	gboolean ret = TRUE;
+	create_file_to_check_memory_address ();
+	if(file_check_valid_memory < 0) {
+		return TRUE;
+	}
+	write (file_check_valid_memory,  (gpointer)addr, 1);
+	if (errno == EFAULT) {
+		ret = FALSE;
+	}
+#else
+	int i = 0;
+	gboolean ret = FALSE;
+	__try {
+		for (i = 0; i < size; i++)
+			*((volatile char*)addr+i);
+		ret = TRUE;
+	} __except(1) {
+		return ret;
+	}
+#endif
+	return ret;
+}
+
 static ErrorCode
 pointer_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 {
@@ -11718,6 +11776,9 @@ pointer_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 	gint64 addr;
 	MonoClass* klass;
 	MonoDomain* domain = NULL;
+	MonoType *type = NULL;
+	int align;
+	int size = 0;
 
 	switch (command) {
 	case CMD_POINTER_GET_VALUE:
@@ -11729,8 +11790,13 @@ pointer_commands (int command, guint8 *p, guint8 *end, Buffer *buf)
 		if (mono_class_get_type (klass)->type != MONO_TYPE_PTR)
 			return ERR_INVALID_ARGUMENT;
 
-		buffer_add_value (buf, mono_class_get_type (mono_class_get_element_class (klass)), (gpointer)addr, domain);
+		type =  mono_class_get_type (mono_class_get_element_class(klass));
+		size = mono_type_size (type, &align);
+		
+		if (!valid_memory_address((gpointer)addr, size))
+			return ERR_INVALID_ARGUMENT;
 
+		buffer_add_value (buf, type, (gpointer)addr, domain);
 		break;
 	default:
 		return ERR_NOT_IMPLEMENTED;