Merge pull request #1324 from Unity-Technologies/unity-master-update-pe-verifier

UnityAlex · web-flow · commit 03751d59a728 · 2020-07-29T09:58:34.000-04:00
Unity master update pe verifier
diff --git a/mono/metadata/image.c b/mono/metadata/image.c
@@ -1345,7 +1345,10 @@ do_mono_image_load (MonoImage *image, MonoImageOpenStatus *status,
 invalid_image:
 	if (errors) {
 		MonoVerifyInfo *info = (MonoVerifyInfo *)errors->data;
-		g_warning ("Could not load image %s due to %s", image->name, info->message);
+		char* log_message = g_strdup_printf("Could not load image %s due to %s\nRun the peverify utility against this for more information.", image->name, info->message);
+		if (!mono_unity_log_error_to_editor(log_message))
+			g_warning (log_message);
+		g_free(log_message);
 		mono_free_verify_list (errors);
 	}
 	MONO_PROFILER_RAISE (image_failed, (image));
diff --git a/mono/metadata/metadata-verify.c b/mono/metadata/metadata-verify.c
@@ -36,9 +36,7 @@
 #ifndef DISABLE_VERIFIER
 /*
  TODO add fail fast mode
- TODO add PE32+ support
  TODO verify the entry point RVA and content.
- TODO load_section_table and load_data_directories must take PE32+ into account
  TODO add section relocation support
  TODO verify the relocation table, since we really don't use, no need so far.
  TODO do full PECOFF resources verification 
@@ -231,9 +229,11 @@ typedef struct {
 	gboolean report_warning;
 	int stage;
 
+	// Mono really only requires 15 here, but verifies the extra is zeroed.
 	DataDirectory data_directories [16];
 	guint32 section_count;
 	SectionHeader *sections;
+	guint pe64; // short name for PE32+; actual PE64 proposal was rejected
 
 	OffsetAndSize metadata_streams [5]; //offset from begin of the image
 } VerifyContext;
@@ -441,8 +441,6 @@ verify_pe_header (VerifyContext *ctx)
 
 	if (offset > ctx->size - 20)
 		ADD_ERROR (ctx, g_strdup ("File with truncated pe header"));
-	if (read16 (pe_header) != 0x14c)
-		ADD_ERROR (ctx, g_strdup ("Invalid PE header Machine value"));
 }
 
 static void
@@ -462,40 +460,58 @@ verify_pe_optional_header (VerifyContext *ctx)
 	if (offset > ctx->size - header_size || header_size > ctx->size)
 		ADD_ERROR (ctx, g_strdup ("Invalid PE optional header size"));
 
-	if (read16 (pe_optional_header) == 0x10b) {
-		if (header_size != 224)
-			ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header size %d", header_size));
-
-		/* LAMESPEC MS plays around this value and ignore it during validation
-		if (read32 (pe_optional_header + 28) != 0x400000)
-			ADD_ERROR (ctx, g_strdup_printf ("Invalid Image base %x", read32 (pe_optional_header + 28)));*/
-		if (read32 (pe_optional_header + 32) != 0x2000)
-			ADD_ERROR (ctx, g_strdup_printf ("Invalid Section Aligmnent %x", read32 (pe_optional_header + 32)));
-		file_alignment = read32 (pe_optional_header + 36);
-		if (file_alignment != 0x200 && file_alignment != 0x1000)
-			ADD_ERROR (ctx, g_strdup_printf ("Invalid file Aligmnent %x", file_alignment));
-		/* All the junk in the middle is irrelevant, specially for mono. */
-		if (read32 (pe_optional_header + 92) > 0x10)
-			ADD_ERROR (ctx, g_strdup_printf ("Too many data directories %x", read32 (pe_optional_header + 92)));
-	} else {
-		if (read16 (pe_optional_header) == 0x20B)
-			ADD_ERROR (ctx, g_strdup ("Metadata verifier doesn't handle PE32+"));
-		else
-			ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header magic %d", read16 (pe_optional_header)));
-	}
+	const guint16 magic = read16 (pe_optional_header);
+
+	if (magic == 0x20B) {
+		// Some fields are the same location for PE32 and PE32+.
+		// A few are offset by 4, 8, or 12, but we do not use them.
+		// Others are offset by 16.
+		// Some are missing.
+		ctx->pe64 = 16;
+	} else if (magic != 0x10b)
+		ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header magic %d", magic));
+
+	// Much of this is over-verification.
+	//
+	// File align and section align do not matter to Mono.
+	//
+	// Mono requires at least 15 data directories. More than that are ignored,
+	// except to require zeros in the 16th.
+	//
+	// Mono requires at least 216 (or pe64:232) size.
+
+	/* LAMESPEC MS plays around this value and ignore it during validation
+	if (read32 (pe_optional_header + 28) != 0x400000)
+	ADD_ERROR (ctx, g_strdup_printf ("Invalid Image base %x", read32 (pe_optional_header + 28)));*/
+	if (read32 (pe_optional_header + 32) != 0x2000)
+		ADD_ERROR (ctx, g_strdup_printf ("Invalid Section Aligmnent %x", read32 (pe_optional_header + 32)));
+	file_alignment = read32 (pe_optional_header + 36);
+	if (file_alignment != 0x200 && file_alignment != 0x1000)
+		ADD_ERROR (ctx, g_strdup_printf ("Invalid file Aligmnent %x", file_alignment));
+	/* All the junk in the middle is irrelevant, specially for mono. */
+
+	if (header_size != 224 + ctx->pe64)
+		ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header size %d", header_size));
+
+	const guint number_of_rvas_and_sizes = read32 (pe_optional_header + 92 + ctx->pe64);
+
+	// Data directories beyond 15 do not matter to mono.
+	if (number_of_rvas_and_sizes > 0x10)
+		ADD_ERROR (ctx, g_strdup_printf ("Too many data directories %x", number_of_rvas_and_sizes));
 }
 
 static void
 load_section_table (VerifyContext *ctx)
 {
 	int i;
 	SectionHeader *sections;
-	guint32 offset =  pe_header_offset (ctx);
+	guint32 offset = pe_header_offset (ctx);
 	const char *ptr = ctx->data + offset;
 	guint16 num_sections = ctx->section_count = read16 (ptr + 2);
 
-	offset += 244;/*FIXME, this constant is different under PE32+*/
-	ptr += 244;
+	const guint optional_header_size = read16 (ptr + 16);
+	offset += optional_header_size + 20;
+	ptr += optional_header_size + 20;
 
 	if (num_sections * 40 > ctx->size - offset)
 		ADD_ERROR (ctx, g_strdup ("Invalid PE optional header size"));
@@ -549,19 +565,18 @@ is_valid_data_directory (int i)
 static void
 load_data_directories (VerifyContext *ctx)
 {
-	guint32 offset =  pe_header_offset (ctx) + 116; /*FIXME, this constant is different under PE32+*/
+	guint32 offset = pe_header_offset (ctx) + 116 + ctx->pe64;
 	const char *ptr = ctx->data + offset;
 	int i;
 
-	for (i = 0; i < 16; ++i) {
+	for (i = 0; i < 16; ++i, ptr += 8) {
 		guint32 rva = read32 (ptr);
 		guint32 size = read32 (ptr + 4);
 
 		/*LAMESPEC the authenticode data directory format is different. We don't support CAS, so lets ignore for now.*/
-		if (i == CERTIFICATE_TABLE_IDX) {
-			ptr += 8;
+		if (i == CERTIFICATE_TABLE_IDX)
 			continue;
-		}
+
 		if ((rva != 0 || size != 0) && !is_valid_data_directory (i))
 			ADD_ERROR (ctx, g_strdup_printf ("Invalid data directory %d", i));
 
@@ -571,8 +586,6 @@ load_data_directories (VerifyContext *ctx)
 		ctx->data_directories [i].rva = rva;
 		ctx->data_directories [i].size = size;
 		ctx->data_directories [i].translated_offset = translate_rva (ctx, rva);
-
-		ptr += 8;
 	}
 }
 
@@ -597,12 +610,8 @@ verify_hint_name_table (VerifyContext *ctx, guint32 import_rva, const char *tabl
 	g_assert (hint_table_rva != INVALID_OFFSET);
 	ptr = ctx->data + hint_table_rva + 2;
 
-	if (memcmp ("_CorExeMain", ptr, SIZE_OF_CORMAIN) && memcmp ("_CorDllMain", ptr, SIZE_OF_CORMAIN)) {
-		char name[SIZE_OF_CORMAIN];
-		memcpy (name, ptr, SIZE_OF_CORMAIN);
-		name [SIZE_OF_CORMAIN - 1] = 0;
-		ADD_ERROR (ctx, g_strdup_printf ("Invalid Hint / Name: '%s'", name));
-	}
+	if (memcmp ("_CorExeMain", ptr, SIZE_OF_CORMAIN) && memcmp ("_CorDllMain", ptr, SIZE_OF_CORMAIN))
+		ADD_ERROR (ctx, g_strdup_printf ("Invalid Hint / Name: '%s'", ptr));
 }
 
 static void
@@ -613,6 +622,10 @@ verify_import_table (VerifyContext *ctx)
 	const char *ptr = ctx->data + offset;
 	guint32 name_rva, ilt_rva, iat_rva;
 
+	// Having no import table is structurally valid
+	if (it.rva == 0 && it.size == 0)
+		return;
+
 	g_assert (offset != INVALID_OFFSET);
 
 	if (it.size < 40)
@@ -640,12 +653,8 @@ verify_import_table (VerifyContext *ctx)
 		g_assert (name_rva != INVALID_OFFSET);
 		ptr = ctx->data + name_rva;
 	
-		if (memcmp ("mscoree.dll", ptr, SIZE_OF_MSCOREE)) {
-			char name[SIZE_OF_MSCOREE];
-			memcpy (name, ptr, SIZE_OF_MSCOREE);
-			name [SIZE_OF_MSCOREE - 1] = 0;
-			ADD_ERROR (ctx, g_strdup_printf ("Invalid Import Table Name: '%s'", name));
-		}
+		if (memcmp ("mscoree.dll", ptr, SIZE_OF_MSCOREE))
+			ADD_ERROR (ctx, g_strdup_printf ("Invalid Import Table Name: '%s'", ptr));
 	}
 	
 	if (ilt_rva) {
diff --git a/mono/metadata/unity-utils.c b/mono/metadata/unity-utils.c
@@ -976,6 +976,23 @@ MONO_API int mono_unity_gc_is_disabled()
 #endif	
 }
 
+// Logging
+static UnityLogErrorCallback editorLoggingCallback;
+MONO_API void mono_unity_set_editor_logging_callback(UnityLogErrorCallback callback)
+{
+	editorLoggingCallback = callback;
+}
+
+gboolean mono_unity_log_error_to_editor(const char *message)
+{
+	if (editorLoggingCallback)
+	{
+		editorLoggingCallback(message);
+		return TRUE;
+	}
+	return FALSE;
+}
+
 MONO_API void 
 mono_unity_install_unitytls_interface(unitytls_interface_struct* callbacks)
 {
diff --git a/mono/metadata/unity-utils.h b/mono/metadata/unity-utils.h
@@ -169,6 +169,10 @@ MONO_API void mono_unity_gc_disable();
 // Deprecated. Remove when Unity has switched to mono_unity_gc_set_mode
 MONO_API int mono_unity_gc_is_disabled();
 
+// logging
+typedef void (*UnityLogErrorCallback) (const char *message);
+MONO_API void mono_unity_set_editor_logging_callback(UnityLogErrorCallback callback);
+gboolean mono_unity_log_error_to_editor(const char *message);
 
 //misc
 MonoAssembly* mono_unity_assembly_get_mscorlib();
