Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A bug in CopySections()? #87

Open
JitCompiler opened this issue Sep 3, 2018 · 1 comment
Open

A bug in CopySections()? #87

JitCompiler opened this issue Sep 3, 2018 · 1 comment

Comments

@JitCompiler
Copy link

JitCompiler commented Sep 3, 2018

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
Copy link

Elmue commented Jun 22, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants