Skip to content

Commit 8b9f5ba

Browse files
jaykrellUnityAlex
authored andcommitted
Teach metadata-verify.c about PE32+. Remove extra string copies. (mono#8224)
* Remove extra string copies. * Teach metadata-verify.c about PE32+.
1 parent 319274d commit 8b9f5ba

File tree

1 file changed

+52
-45
lines changed

1 file changed

+52
-45
lines changed

mono/metadata/metadata-verify.c

Lines changed: 52 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@
3636
#ifndef DISABLE_VERIFIER
3737
/*
3838
TODO add fail fast mode
39-
TODO add PE32+ support
4039
TODO verify the entry point RVA and content.
41-
TODO load_section_table and load_data_directories must take PE32+ into account
4240
TODO add section relocation support
4341
TODO verify the relocation table, since we really don't use, no need so far.
4442
TODO do full PECOFF resources verification
@@ -231,9 +229,11 @@ typedef struct {
231229
gboolean report_warning;
232230
int stage;
233231

232+
// Mono really only requires 15 here, but verifies the extra is zeroed.
234233
DataDirectory data_directories [16];
235234
guint32 section_count;
236235
SectionHeader *sections;
236+
guint pe64; // short name for PE32+; actual PE64 proposal was rejected
237237

238238
OffsetAndSize metadata_streams [5]; //offset from begin of the image
239239
} VerifyContext;
@@ -460,40 +460,58 @@ verify_pe_optional_header (VerifyContext *ctx)
460460
if (offset > ctx->size - header_size || header_size > ctx->size)
461461
ADD_ERROR (ctx, g_strdup ("Invalid PE optional header size"));
462462

463-
if (read16 (pe_optional_header) == 0x10b) {
464-
if (header_size != 224)
465-
ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header size %d", header_size));
466-
467-
/* LAMESPEC MS plays around this value and ignore it during validation
468-
if (read32 (pe_optional_header + 28) != 0x400000)
469-
ADD_ERROR (ctx, g_strdup_printf ("Invalid Image base %x", read32 (pe_optional_header + 28)));*/
470-
if (read32 (pe_optional_header + 32) != 0x2000)
471-
ADD_ERROR (ctx, g_strdup_printf ("Invalid Section Aligmnent %x", read32 (pe_optional_header + 32)));
472-
file_alignment = read32 (pe_optional_header + 36);
473-
if (file_alignment != 0x200 && file_alignment != 0x1000)
474-
ADD_ERROR (ctx, g_strdup_printf ("Invalid file Aligmnent %x", file_alignment));
475-
/* All the junk in the middle is irrelevant, specially for mono. */
476-
if (read32 (pe_optional_header + 92) > 0x10)
477-
ADD_ERROR (ctx, g_strdup_printf ("Too many data directories %x", read32 (pe_optional_header + 92)));
478-
} else {
479-
if (read16 (pe_optional_header) == 0x20B)
480-
ADD_ERROR (ctx, g_strdup ("Metadata verifier doesn't handle PE32+"));
481-
else
482-
ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header magic %d", read16 (pe_optional_header)));
483-
}
463+
const guint16 magic = read16 (pe_optional_header);
464+
465+
if (magic == 0x20B) {
466+
// Some fields are the same location for PE32 and PE32+.
467+
// A few are offset by 4, 8, or 12, but we do not use them.
468+
// Others are offset by 16.
469+
// Some are missing.
470+
ctx->pe64 = 16;
471+
} else if (magic != 0x10b)
472+
ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header magic %d", magic));
473+
474+
// Much of this is over-verification.
475+
//
476+
// File align and section align do not matter to Mono.
477+
//
478+
// Mono requires at least 15 data directories. More than that are ignored,
479+
// except to require zeros in the 16th.
480+
//
481+
// Mono requires at least 216 (or pe64:232) size.
482+
483+
/* LAMESPEC MS plays around this value and ignore it during validation
484+
if (read32 (pe_optional_header + 28) != 0x400000)
485+
ADD_ERROR (ctx, g_strdup_printf ("Invalid Image base %x", read32 (pe_optional_header + 28)));*/
486+
if (read32 (pe_optional_header + 32) != 0x2000)
487+
ADD_ERROR (ctx, g_strdup_printf ("Invalid Section Aligmnent %x", read32 (pe_optional_header + 32)));
488+
file_alignment = read32 (pe_optional_header + 36);
489+
if (file_alignment != 0x200 && file_alignment != 0x1000)
490+
ADD_ERROR (ctx, g_strdup_printf ("Invalid file Aligmnent %x", file_alignment));
491+
/* All the junk in the middle is irrelevant, specially for mono. */
492+
493+
if (header_size != 224 + ctx->pe64)
494+
ADD_ERROR (ctx, g_strdup_printf ("Invalid optional header size %d", header_size));
495+
496+
const guint number_of_rvas_and_sizes = read32 (pe_optional_header + 92 + ctx->pe64);
497+
498+
// Data directories beyond 15 do not matter to mono.
499+
if (number_of_rvas_and_sizes > 0x10)
500+
ADD_ERROR (ctx, g_strdup_printf ("Too many data directories %x", number_of_rvas_and_sizes));
484501
}
485502

486503
static void
487504
load_section_table (VerifyContext *ctx)
488505
{
489506
int i;
490507
SectionHeader *sections;
491-
guint32 offset = pe_header_offset (ctx);
508+
guint32 offset = pe_header_offset (ctx);
492509
const char *ptr = ctx->data + offset;
493510
guint16 num_sections = ctx->section_count = read16 (ptr + 2);
494511

495-
offset += 244;/*FIXME, this constant is different under PE32+*/
496-
ptr += 244;
512+
const guint optional_header_size = read16 (ptr + 16);
513+
offset += optional_header_size + 20;
514+
ptr += optional_header_size + 20;
497515

498516
if (num_sections * 40 > ctx->size - offset)
499517
ADD_ERROR (ctx, g_strdup ("Invalid PE optional header size"));
@@ -547,19 +565,18 @@ is_valid_data_directory (int i)
547565
static void
548566
load_data_directories (VerifyContext *ctx)
549567
{
550-
guint32 offset = pe_header_offset (ctx) + 116; /*FIXME, this constant is different under PE32+*/
568+
guint32 offset = pe_header_offset (ctx) + 116 + ctx->pe64;
551569
const char *ptr = ctx->data + offset;
552570
int i;
553571

554-
for (i = 0; i < 16; ++i) {
572+
for (i = 0; i < 16; ++i, ptr += 8) {
555573
guint32 rva = read32 (ptr);
556574
guint32 size = read32 (ptr + 4);
557575

558576
/*LAMESPEC the authenticode data directory format is different. We don't support CAS, so lets ignore for now.*/
559-
if (i == CERTIFICATE_TABLE_IDX) {
560-
ptr += 8;
577+
if (i == CERTIFICATE_TABLE_IDX)
561578
continue;
562-
}
579+
563580
if ((rva != 0 || size != 0) && !is_valid_data_directory (i))
564581
ADD_ERROR (ctx, g_strdup_printf ("Invalid data directory %d", i));
565582

@@ -569,8 +586,6 @@ load_data_directories (VerifyContext *ctx)
569586
ctx->data_directories [i].rva = rva;
570587
ctx->data_directories [i].size = size;
571588
ctx->data_directories [i].translated_offset = translate_rva (ctx, rva);
572-
573-
ptr += 8;
574589
}
575590
}
576591

@@ -595,12 +610,8 @@ verify_hint_name_table (VerifyContext *ctx, guint32 import_rva, const char *tabl
595610
g_assert (hint_table_rva != INVALID_OFFSET);
596611
ptr = ctx->data + hint_table_rva + 2;
597612

598-
if (memcmp ("_CorExeMain", ptr, SIZE_OF_CORMAIN) && memcmp ("_CorDllMain", ptr, SIZE_OF_CORMAIN)) {
599-
char name[SIZE_OF_CORMAIN];
600-
memcpy (name, ptr, SIZE_OF_CORMAIN);
601-
name [SIZE_OF_CORMAIN - 1] = 0;
602-
ADD_ERROR (ctx, g_strdup_printf ("Invalid Hint / Name: '%s'", name));
603-
}
613+
if (memcmp ("_CorExeMain", ptr, SIZE_OF_CORMAIN) && memcmp ("_CorDllMain", ptr, SIZE_OF_CORMAIN))
614+
ADD_ERROR (ctx, g_strdup_printf ("Invalid Hint / Name: '%s'", ptr));
604615
}
605616

606617
static void
@@ -638,12 +649,8 @@ verify_import_table (VerifyContext *ctx)
638649
g_assert (name_rva != INVALID_OFFSET);
639650
ptr = ctx->data + name_rva;
640651

641-
if (memcmp ("mscoree.dll", ptr, SIZE_OF_MSCOREE)) {
642-
char name[SIZE_OF_MSCOREE];
643-
memcpy (name, ptr, SIZE_OF_MSCOREE);
644-
name [SIZE_OF_MSCOREE - 1] = 0;
645-
ADD_ERROR (ctx, g_strdup_printf ("Invalid Import Table Name: '%s'", name));
646-
}
652+
if (memcmp ("mscoree.dll", ptr, SIZE_OF_MSCOREE))
653+
ADD_ERROR (ctx, g_strdup_printf ("Invalid Import Table Name: '%s'", ptr));
647654
}
648655

649656
if (ilt_rva) {

0 commit comments

Comments
 (0)