Description
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).
- When
sec_sizeis 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. - 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.
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