[WIP] CFG property page for Peview

[lucasg]

Hi,

I've worked for some time now on a plugin analyzing CFG and its whitelist mechanism. In the process, I've tweaked peview in order to show me the list of authorized functions as indirect call in a given PE :

It cleary replicate the __guard_fids_table name entry in IDA :

The code has probably some bugs left in it but if you guys are interested in merging this feature into the main tree, I will gladly iron out the remaining kinks.

L.

[dmex]

Very cool, thanks! I'll merge the code when you think it's ready.

[lucasg]

Ok I've cleaned up my code and fixed the several bugs I've spotted, so it's good to go on my part.
(There may still be some memory leaks since I've not yet fully understood the reference counting mechanism for PPH_STRINGS).
Feel free to ask for explanations or corrections if you don't understand some parts of my code (it's basic parsing stuff though).

The last commit in this PR also add the CFG configuration items in the load config property page.

L.

[dmex]

I've fixed most issues and memory leaks, there's just one issue remaining with how symbol loading is done that I will fix up later... Can you please double check my changes and make sure I haven't accidentally broken something with CFG parsing? 😄

You can review the changes here:
e3bf064

Here's some notes from those changes:

• AutoPool functions - I've changed peview to use PhModalPropertySheet instead of the Win32 PropertySheet function since it manages an AutoPool for property sheets - meaning you can now use Pha_XXX functions (e.g PhaFormatString) anywhere inside peview without having to create an AutoPool.

• Tabspace characters - You need to change your C/C++ settings to use spaces instead of tabs.

• Function naming - don't use PH_XXX names for functions or structures outside of phlib or for native types used by the operating system or it becomes a pain to analyse dump files after a crash. For example the PPH_MAPPED_IMAGE_CFG_ENTRY structure is a native Windows structure and shouldn't start with PPH_XXX.

• camelCase naming - Use camelCase for variable names inside functions.

• Types - don't use CRT types like size_t or hungarian types like DWORD and use Windows NT types SIZE_T and ULONG.

You can review project guidelines on the link below. I'm very flexible when it comes to the guidelines so don't worry about them too much 😄
https://github.com/processhacker2/processhacker2/blob/master/HACKING.md

Also, the PhGetMappedImageCfgEntry function - The Reserved field of the structure used by GuardFunctionTable seems to have some undocumented flags that are currently being used? :)

[lucasg]

I've pulled your modifications and I've spotted a regression : a IMAGE_CFG_ENTRY consist of a RVA (stored as 32 bit integer) and an optional header (containing SuppressedCall for example) whose size is described by the IMAGE_GUARD_CF_FUNCTION_TABLE_SIZE_MASK bitmask from GuardFlags. So a CFG entry is at least 4 bytes large.

the line in mapimg.c:1233 :
CfgConfig->EntrySize = (ULONG)((config64->GuardFlags & IMAGE_GUARD_CF_FUNCTION_TABLE_SIZE_MASK) >> IMAGE_GUARD_CF_FUNCTION_TABLE_SIZE_SHIFT);
should be :
CfgConfig->EntrySize = sizeof(FIELD_OFFSET(IMAGE_CFG_ENTRY, Rva)) + (ULONG)((config64->GuardFlags & IMAGE_GUARD_CF_FUNCTION_TABLE_SIZE_MASK) >> IMAGE_GUARD_CF_FUNCTION_TABLE_SIZE_SHIFT);

Since it's a one-liner fix, I will let you do it on your end (it will be easier than creating a whole PR for it).

Regarding the code style fixes, I've nothing to correct. I probably should have read the HACKING.md file before submitted this PR ...

And to answer your last question, I haven't noticed other use of the Reserved field yet in any binaries I've analyzed. Same for the GuardAddressTakenIatEntryTable and GuardLongJumpTable table entries in load configuration. I should mention that even the SuppressedCall flag is not officially documented, I just named it after the RtlpGuardIsSuppressedImageRva function in ntdll since it used there :p

[dmex]

Thanks, I fixed the regression 👍

I've also added window resize support to peview in PR #102 and I'm just waiting on a review from @wj32 before I can merge the code - It'll make using peview a lot more user friendly 😃

The reserved field is definitely being used on my system (Win10 x64, build 14393)... For example the following DLLs are showing values stored in the 'Reserved' field:
"C:\Windows\System32\ztrace_maps.dll"
"C:\Windows\System32\xmllite.dll"

Seems to be used by nearly all DLLs other than ntdll, kernelbase and kernel32?

[dmex]

Resize support for peview has been merged 👍

Binaries such as Visual Studio have values stored in the GuardAddressTakenIatEntryTable and GuardLongJumpTable fields?
C:\Program Files (x86)\Microsoft Visual Studio 14.0\Common7\IDE\devenv.exe

[lucasg]

Hmm, I've taken a look at it and the values seemed fishy to me. There is definitively a bug here.

The IMAGE_LOAD_CONFIG_DIRECTORYXX struct's first field is the instance's own size. In the case of devenv.exe the load config data directory does not define any CFG configuration (since it's a Wow64 executable) and set the size to FIELD_OFFSET(IMAGE_LOAD_CONFIG_DIRECTORY32, GuardAddressTakenIatEntryTable) : we should not try to parse beyond.

Since PhGetMappedImageLoadConfigXX is a simple cast over the mapped data, we need to check the load config size every time we want to read one of its field member.

Here the diff : loadconfig.diff.txt

[dmex]

Thanks, I've merged the changes in commit f0517b3

Btw, the nightly builds now include the peview changes:
https://wj32.org/processhacker/nightly.php

[lucasg]

Okay great. If that's too not much to ask, is it possible to exports some functions from mapimg in the ProcessHacker.lib ? That way I can use it in a plugin.

For my use case I would need the following functions exported :

• PhGetMappedImageLoadConfig64
• PhGetMappedImageLoadConfig32
• PhGetMappedImageCfg
• PhGetMappedImageCfgEntry
• PhInitializeMappedImage
• PhLoadMappedImage

L.

[dmex]

Done