yara icon indicating copy to clipboard operation
yara copied to clipboard

The "pe_get_directory_entry" doesn't take NumberOfRvaAndSizes into account

Open ladislav-zezula opened this issue 3 years ago • 5 comments

@wxsBSD: The condition in pe_utils.c(133) is wrong.

Explanation for Resources

In Windows, any access to data directories is controlled by the RtlImageDirectoryEntryToData function. This one first checks whether the desired directory entry is smaller than NumberOfRvaAndSizes. If it's not, it means that the requested directory entry is not there.

Samples: f008f983a6ba1743600f64596aa41b5ce2ed217e0c4ef1143bd690cf9008dd7a NumberOfRvaAndSizes is 2, the file has no resources, Windows display no icon 32c06152e18470b8fdb6d45d7497430c07d796cbc49604d357194c9761e51171 NumberOfRvaAndSizes = 0x10, aka "standard" PE file. Windows display icon from the PE 29eeeecf2c458ea3da1ce9d6d54742c0fad490cb2165f371f53b61941eedf072 NumberOfRvaAndSizes is 0x20, yet the sample runs perfectly. Windows display icon from the PE

Explanation for .NET

In pe_utils.c(122), there's explanation about .NET ignoring NumberOfRvaAndSizes. The steps leading to execution of a .NET code are here (sample: 7ff1bf680c80fd73c0b35084904848b3705480ddeb6d0eff62180bd14cd18570):

  1. In the loader (ntdll!LdrpInitializeProcess), the code checks for presence of IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR. If it's present, then image entry point is changed to mscoree!_CorExeMain or mscoree!_CorDllMain.
  2. Because NumberOfRvaAndSizes == 0x0B (in the sample above), the loader assumes that it's not a .NET binary. In that case, no tampering of entry point address is done and the image runs as if it was non-.NET
  3. Because 32-bit .NET binaries are required to have standard entry point (see below), the code jumps to mscoree!_CorExeMain nonetheless. Integrity checks are done in order to verify that it really is a .NET image, but NumberOfRvaAndSizes is ignored.
0043F9FE                                         public start
0043F9FE                         start           proc near
0043F9FE FF 25 D8 12 44 00                       jmp     ds:_CorExeMain
0043F9FE                         start           endp

The sequence of steps doesn't work for 64-bit images - they do NOT have any code at entry point.

TLDR:

  1. The function pe_get_directory_entry() must NOT check whether NumberOfRvaAndSizes > 0x10
  2. The function must check for NumberOfRvaAndSizes < DataDirectory except for 0x0E for 32-bit binary
  3. The function must NOT check for SizeOfOptionalHeader. The only thing that Windows use SizeOfOptionalHeader for is to get offset of the section headers. NOTHING ELSE.

ladislav-zezula avatar Jul 02 '21 11:07 ladislav-zezula

@wxsBSD did you have a chance to review this? If you don't have additional comments I'll merge this PR.

plusvic avatar Aug 17 '21 08:08 plusvic

I'll take a look at this in the next few days. Sorry for the delay.

wxsBSD avatar Aug 19 '21 20:08 wxsBSD

Thank you so much for making this more robust - and now I understand more about what you were talking about on twitter a few months ago! This makes a lot more sense now, and looking back on commit efd981848 I can see exactly what you are talking about.

Out of curiosity, what generates 32bit .NET binaries that are like this? Is this normal VS compiler behavior?

wxsBSD avatar Aug 25 '21 01:08 wxsBSD

Out of curiosity, what generates 32bit .NET binaries that are like this? Is this normal VS compiler behavior?

Absolutely not. Compilers always put 0x10 into NumberOfRvaAndSizes. The only samples that have different value are some exotic proof-of-concepts, like those from Mr. Corkami.

We came across a sample once when we were checking YARA output versus the retdec-fileinfo tool. Whilst retdec-fileinfo showed it's a .NET, YARA didn't. The sample ran just fine, tho - as .NET.

We investigated the discrepancy and found out that bug/feature about NumberOfRvaAndSizes. Took me some time to figure out why does it actually work.

ladislav-zezula avatar Aug 25 '21 05:08 ladislav-zezula

Awesome! And thanks again for these contributions. I have always tried to err on the side of "if windows loads this, we should parse it" but in this particular case only had the docs to go on which says the value must be set.

wxsBSD avatar Aug 25 '21 16:08 wxsBSD