Wrong offset used when checking Version string of .net metadata

[dangodangodango]

yara/libyara/modules/dotnet/dotnet.c

Lines 1710 to 1728 in ee78353

	offset = metadata_root = pe_rva_to_offset(
	pe, yr_le32toh(cli_header->MetaData.VirtualAddress));
	if (!struct_fits_in_pe(pe, pe->data + offset, NET_METADATA))
	return;
	metadata = (PNET_METADATA) (pe->data + offset);
	// Version length must be between 1 and 255, and be a multiple of 4.
	// Also make sure it fits in pe.
	md_len = yr_le32toh(metadata->Length);
	if (md_len == 0 || md_len > 255 || md_len % 4 != 0 ||
	!fits_in_pe(pe, pe->data + offset, md_len))
	{
	return;
	}

yara/libyara/modules/dotnet/dotnet.c

Lines 1637 to 1657 in ee78353

	int64_t metadata_root = pe_rva_to_offset(
	pe, yr_le32toh(cli_header->MetaData.VirtualAddress));
	if (!struct_fits_in_pe(pe, pe->data + metadata_root, NET_METADATA))
	return false;
	NET_METADATA* metadata = (NET_METADATA*) (pe->data + metadata_root);
	if (yr_le32toh(metadata->Magic) != NET_METADATA_MAGIC)
	return false;
	// Version length must be between 1 and 255, and be a multiple of 4.
	// Also make sure it fits in pe.
	uint32_t md_len = yr_le32toh(metadata->Length);
	if (md_len == 0 || md_len > 255 || md_len % 4 != 0 ||
	!fits_in_pe(pe, pe->data + offset, md_len))
	{
	return false;
	}

The above two places (line 1653 and line 1724) are checking whether metadata version string is fits in pe, Version string offset is pe->data + offset + sizeof(NET_METADATA), and size is md_len. But the above code checks the md_len bytes starting from offset (which is metadata_root)

The correct code should be as follows:

!fits_in_pe(pe, pe->data + offset + sizeof(NET_METADATA), md_len)

And in the function dotnet_is_dotnet, the variable offset is not updated to metadata_root, so in line 1653, offset is still the offset of CLI_HEADER, this is also a problem.

[dangodangodango]

yara/libyara/modules/dotnet/dotnet.c

Lines 1658 to 1676 in ee78353

	if (IS_64BITS_PE(pe))
	{
	if (yr_le16toh(OptionalHeader(pe, NumberOfRvaAndSizes)) <
	IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR)
	return false;
	}
	else if (!(pe->header->FileHeader.Characteristics & IMAGE_FILE_DLL)) // 32bit
	{
	// Check first 2 bytes of the Entry point are equal to 0xFF 0x25
	int64_t entry_offset = pe_rva_to_offset(
	pe, yr_le32toh(pe->header->OptionalHeader.AddressOfEntryPoint));
	if (offset < 0 || !fits_in_pe(pe, pe->data + entry_offset, 2))
	return false;
	const uint8_t* entry_data = pe->data + entry_offset;
	if (!(entry_data[0] == 0xFF && entry_data[1] == 0x25))
	return false;
	}

And in line 1670, shouldn't it be entry_offset < 0 instead of offset < 0?

[wxsBSD]

I think you're right. Any chance you plan to submit dangodangodango@ac71504 as a PR? I think it is the correct fix (but I haven't tested it to be honest).