fancycode / MemoryModule

Library to load a DLL from memory.
http://www.joachim-bauch.de/tutorials/loading-a-dll-from-memory/
Mozilla Public License 2.0
2.8k stars 755 forks source link

A bug in CopySections()? #87

Open JitCompiler opened 6 years ago

JitCompiler commented 6 years ago

In this function under MemoryModule.c:

static BOOL
CopySections(const unsigned char *data, size_t size, PIMAGE_NT_HEADERS old_headers, PMEMORYMODULE module)
{
...
PIMAGE_SECTION_HEADER section = IMAGE_FIRST_SECTION(module->headers);
for (..) {
  if (section->SizeOfRawData == 0)
  ...
}

It looks like this line:

PIMAGE_SECTION_HEADER section = IMAGE_FIRST_SECTION(module->headers); should instead be this or something else:

PIMAGE_SECTION_HEADER section = IMAGE_FIRST_SECTION(old_headers);

This function is called like this:

headers = (unsigned char *)allocMemory(code,
        old_header->OptionalHeader.SizeOfHeaders,
        MEM_COMMIT,
        PAGE_READWRITE,
        userdata);
...
result->headers = (PIMAGE_NT_HEADERS)&((const unsigned char *)(headers))[dos_header->e_lfanew];
..
if (!CopySections((const unsigned char *) data, size, old_header, result)) {

Hence, the section variable is initialized to the SECTION_HEADER in output buffer (module->headers) we just allocated. Later, we do an if check using if (section->SizeOfRawData == 0). The problem is, the output buffer must be zero right after allocation by VirtualAlloc(). Even worse is there are something else performed if the output buffer is not zero which won't be executed at all. So it looks like the code is not behaving in a manner consistent with its initialized value.

Since I am not able to fully understand what these lines are doing. I can only second guess that the initializer is incorrect.

Elmue commented 4 years ago

The only variables which are used from the structure in the variable 'section' are section->SizeOfRawData and section->VirtualAddress

It does not matter if you load 'section' from module->headers or from old_headers because before calling CopySections() the following line is executed: memcpy(headers, dos_header, old_header->OptionalHeader.SizeOfHeaders);

So they are always the same.