[Bug] Access Violation bugs & Integer Overflows

[NapongiZero]

Hey hasherzade, I'm a big fan of your tools. I've done a bit of fuzzing & found some bugs.

Machine specs

Windows 10 Pro, 21H2, ASLR-off

Description

Basically, I've ran a modified version of your test project project_template that uses libpeconv (32 bit), with different inputs until I received a few crashes.

I've analyzed the crashes and then classified them into two groups based on behavior.

Crash 1

Reproduction: The crash is reproducible in 100% of executions.

To reproduce, use "id_000001_00_EXCEPTION_ACCESS_VIOLATION".

Crash type: An access violation is caused at pe_raw_to_virtual.cpp in function sections_virtual_to_raw at line 95 -> memcpy(section_raw_ptr, section_mapped, sec_size);.

It seems that during the memcpy call, there is an attempt to copy from an unallocated page in memory. Basically, there's an attempt to read beyond heap bounds.

0:000> g
(33b4.3020): Access violation - code c0000005 (!!! second chance !!!)
eax=006df71f ebx=002b8000 ecx=ffffc71f edx=ffffff00 esi=006e3000 edi=001e47e1
eip=6de0395e esp=004ffd9c ebp=004ffe00 iopl=0         nv up ei ng nz na po cy
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00010283
VCRUNTIME140D!memcpy+0x4e:
6de0395e f3a4            rep movs byte ptr es:[edi],byte ptr [esi]

*Note: eax = section_raw_ptr, ecx = section_mapped, edx = sec_size. The bug occurs even when the sec_size isn't unproportionally huge.

Call stack:

**Call stack: (Windbg)**
```windbg
0:000> k
 # ChildEBP RetAddr      
00 00baf9ac 0019db43     VCRUNTIME140D!memcpy+0x4e [d:\a01\_work\11\s\src\vctools\crt\vcruntime\src\string\i386\memcpy.asm @ 194] 
01 00bafa0c 0019d58b     project_template!sections_raw_to_virtual+0x563 [..\libpeconv_tpl\libpeconv\libpeconv\src\pe_raw_to_virtual.cpp @ 94] 
02 00bafa50 0019883b     project_template!peconv::pe_raw_to_virtual+0x15b [..\libpeconv_tpl\libpeconv\libpeconv\src\pe_raw_to_virtual.cpp @ 134] 
03 00bafa7c 001989e0     project_template!peconv::load_pe_module+0xbb [..\libpeconv_tpl\libpeconv\libpeconv\src\pe_loader.cpp @ 50] 
04 00bafa9c 00192be8     project_template!peconv::load_pe_executable+0x20 [..\libpeconv_tpl\libpeconv\libpeconv\src\pe_loader.cpp @ 82] 
05 00bafacc 00192d94     project_template!load_payload+0x78 [..\libpeconv_tpl\project_template\main.cpp @ 39] 
...

Why/When the bug occurs:

There are checks in sections_raw_to_virtual() to make sure that the virtual sections/addresses aren't out of bounds (lines 55,60,64 in pe_raw_to_virtual.cpp), but they aren't able to catch the issue due to an Integer Overflow bug (to be more precise, it's a size_t overflow/wrap-around).

1. When sec_size is too large, (for example 0xffffff00 as in "id_000001_00_EXCEPTION_ACCESS_VIOLATION") it causes an Integer overflow that wraps around 0, and thus the if statement (next_sec->VirtualAddress + sec_size) > destBufferSize) is equals to false. This can occur at least two times, in lines 55 & 64.
2. There's an Integer overflow in line 53, in a redundant variable raw_end. This isn't a big issue since the variable isn't being used anywhere else in the code, but it's a good idea to remove it either way.

### Crash 2 **Reproduction:** The crash is reproducible in 100% of executions. To reproduce, use "id_000002_00_EXCEPTION_ACCESS_VIOLATION". **Crash type:**: A *INVALID_POINTER_READ* occurs at `pe_virtual_to_raw.cpp` in function `sections_virtual_to_raw` at line 47 -> `if (next_sec->PointerToRawData == 0 || next_sec->SizeOfRawData == 0)`. The program tries to read `next_sec->PointerToRawData` but it points to a memory location that doesn't belong to the program's address space. ```cpp 42 for (WORD i = 0; i < fileHdr->NumberOfSections; i++) { 43 PIMAGE_SECTION_HEADER next_sec = (PIMAGE_SECTION_HEADER)((ULONGLONG)secptr + (IMAGE_SIZEOF_SECTION_HEADER * i)); 44 if (!validate_ptr((const LPVOID)payload, destBufferSize, next_sec, IMAGE_SIZEOF_SECTION_HEADER)) { 45 return false; 46 } 47 if (next_sec->PointerToRawData == 0 || next_sec->SizeOfRawData == 0) { // Bug here - read access violation 48 continue; //skipping empty 49 } 50 LPVOID section_mapped = destBuffer + next_sec->VirtualAddress; ``` ## Exploitability & Mitigations In crash 1 we should be able to control the parameters to the memcpy, which may cause a Heap B.O. Either way, you should add a check for Integer overflows (or wrapping around behavior) at the mentioned lines to avoid undefined behavior. At first glance I'd assume the second crush isn't exploitable, but you might want to use Exception handling such as /EHa or SEH when compiling the program. *Edit: You can find the binaries that crashed the application here: https://github.com/NapongiZero/libpeconvCrashes

[hasherezade]

hi @NapongiZero ! that's a great report! thank you so much for your work, and for the quality descriptions! I am gonna fix it soon & notify you.

[hasherezade]

@NapongiZero - please check out my latest changes, and let me know what do you think!

[NapongiZero]

Hi @hasherezade, thank you for the kind words! The changes are great. I ran it through my tests and it looks like it fixed the bugs. Also, that's the fastest bug fix I've ever seen. Kudos to you!