Bug results in crash when loading some DLLs. (with code fix!)

[Elmue]

I tried to load a DLL which is protected with WinLicense into a process.
When calling the entry point I got a crash when Data Execution Prevention was enabled for the process.
I analyzed the problem and found that fancycode has a fat bug which has not yet been reported here.
These bugs are not relevant when you load a "normal" DLL.
But a DLL which is protected with WinLicense has strong anti-debugging techniques built in.
WinLicense modifies the executing code at runtime!
It permanently writes into the code section.

Apart from that I found fancycode (written by Joachim Bauch) is very clumsy and awkward.
There is lot of code which is superfluous and far more complicated than necessary.

For example in CopySections() he calls
memset(dest, 0, section_size);
This is nonsense because VirtualAlloc() already returns memory which is zeroed.

It is also nonsense that he calls VirtualAlloc() first for the entire image of the DLL (in MemoryLoadLibraryEx()) and then he calls VirtualAlloc() again for each section in CopySections() although the memory is already allocated.

It is also unnecessary to call VirtualFree() in FinalizeSection() in the case that a section has the flag IMAGE_SCN_MEM_DISCARDABLE.
I studied how the Microsoft's LoadLibrary() works and it does NOT free sections which are marked as discardable.

But the real fat bug is in FinalizeSections()
When he sets the protection flags in FinalizeSection() he passes the wrong size.
This results in the crash in my WinLicense protected DLL if DEP is enabled.

Correct would be:
VirtualProtect(..., Section->Misc.VirtualSize, ...)
But he uses instead
VirtualProtect(..., GetRealSectionSize(), ...)

This is wrong because the entire section must have the Execute flag, not only the first part where he has copied to with memcpy().
It is wrong because there are DLLs which unpack or modify code at runtime.
If the pages where the code is copied to do not have the Execute flag set, Data Execution Prevention triggers an Access Violation.

I don't post a pull request because nobody cares here since years.
This is a dead project. Joachim Bauch neither merges pull requests nor does he answer emails.
So if you are interested take my code and fix it on your own.

// Copy all sections from file data (pu8_Data) to virtual address in memory
// Returns API error
DWORD CopySections(const BYTE* pu8_Data, DWORD u32_Size, MEMORYMODULE* pk_ModData)
{
    IMAGE_SECTION_HEADER* pk_Section = IMAGE_FIRST_SECTION(pk_ModData->headers); 
    for (int i=0; i<pk_ModData->headers->FileHeader.NumberOfSections; i++, pk_Section++)
    {
        size_t u32_SectSizeAligned = AlignValueUp(pk_Section->Misc.VirtualSize, pk_ModData->pageSize);

        if (u32_Size < pk_Section->PointerToRawData + pk_Section->SizeOfRawData ||
            pk_Section->SizeOfRawData > u32_SectSizeAligned)
            return ERROR_BAD_EXE_FORMAT;

        BYTE* pu8_Dest = pk_ModData->codeBase + pk_Section->VirtualAddress;
        memcpy(pu8_Dest, pu8_Data + pk_Section->PointerToRawData, pk_Section->SizeOfRawData);
    }
    return 0;
}

// Loop through all sections and set the access flags (Read, Write, Execute,...)
// Returns API error
DWORD FinalizeSections(MEMORYMODULE* pk_ModData)
{
    IMAGE_SECTION_HEADER* pk_Section = IMAGE_FIRST_SECTION(pk_ModData->headers);
    for (int i=0; i<pk_ModData->headers->FileHeader.NumberOfSections; i++, pk_Section++) 
    {
        DWORD u32_Size = pk_Section->Misc.VirtualSize;
        if (u32_Size == 0) 
            continue;

        BOOL b_Executable = (pk_Section->Characteristics & IMAGE_SCN_MEM_EXECUTE) != 0;
        BOOL b_Readable   = (pk_Section->Characteristics & IMAGE_SCN_MEM_READ)    != 0;
        BOOL b_Writeable  = (pk_Section->Characteristics & IMAGE_SCN_MEM_WRITE)   != 0;
        if (pk_Section->Characteristics & IMAGE_SCN_CNT_CODE)
        { 
            b_Executable = TRUE; 
            b_Readable   = TRUE; 
        }

        DWORD u32_Protect = ProtectionFlags[b_Executable][b_Readable][b_Writeable];
        if (pk_Section->Characteristics & IMAGE_SCN_MEM_NOT_CACHED) 
            u32_Protect |= PAGE_NOCACHE;

        BYTE* pu8_Address = pk_ModData->codeBase + pk_Section->VirtualAddress;

        DWORD u32_OldProtect;
        if (!gf_VirtualProtect(pu8_Address, u32_Size, u32_Protect, &u32_OldProtect))
            return GetLastError();
    }
    return 0;
}

The following clumsy functions and structures are not needed anymore:

delete: GetRealSectionSize()
delete: FinalizeSection()   // do not confuse with FinalizeSections()
delete: SECTIONFINALIZEDATA

As you see my functions are far shorter than the original code.
And they work without crash!

Also have a look at my further postings here:

Create Activation Context Implementation (important)
#100

Remove unneccessay functions for searching resources:
#53 (comment)

and more...

By the way: The code from Joachim Bauch is not cleanly written.
You find a much cleaner code to load a DLL to memory on Codeproject:
https://www.codeproject.com/Tips/430684/Loading-Win-DLLs-manually-without-LoadLibrary
But it is lacking support for 64 bit Safe SEH and the Activation Context is not implemented in neither project.

[akasandra]

We used "Load dll from memory" long before MemoryModule has come to scene. There always were loads of shitcode, and MemoryModule genetically consists of this legacy. It is not made by someone special, it is a compilation of knowledge that circulated long before - as code pastes, and many people used it does not really care how and why it works. Specifically, everything related to sections and relocation - topics which most people don't really understand. With all this I am just clarifying that it is not a bug but just common lack of knowledge. If what you say is right actually.

The reason this comment is made is because someone around me has forked MemoryModule and as part of his modifications (under heavy usage in different cases), they actually fixed this issue, but never reported it - and this is what happened for years to code like memorymodule. Unfortunatelly I am not sure was the issue exactly FinalizeSections+VirtualProtect usage, but it was something you describe with self-patching code.

[Elmue]

It is definitely a bug. It results in a crash.
And it is wrong to set the flags only for a part of a section instead of the entire section.
But without Data Execution Prevention you don't notice the bug.
And with a "normal" DLL with a code section that never changes you also don't notice the bug.

DEP was introduced with Windows XP SP2 in 2004.
The comment in the source code says:

Copyright (c) 2004-2015 by Joachim Bauch

So it was wrong from the very beginning.

I found several forks of fancycode here on Github. But they contain only minor changes.
None of them fixes this bug. They contain just a copy of the same bug.
Simply search for the function VirtualProtect() and have a look at the size which is passed to it.
If it is section->Misc.VirtualSize it is correct.
If it uses the result from GetRealSectionSize() or section->SizeOfRawData it is wrong.

The only project which I found which does it correctly is not a fork of fancycode:
https://www.codeproject.com/Tips/430684/Loading-Win-DLLs-manually-without-LoadLibrary

topics which most people don't really understand.

It is not difficult to understand how to load a DLL to memory.
The steps are quite logical and straight forward.
The problem is that Joachim Bauch wrote a horrible spaghetti code.
And THAT is real difficulty to understand.
Have a look at the code on Codeproject which is cleanly written.
Do you have problems to understand it?

And the code would be far more easy to understand if there was a usefull TRACE implemented which shows what the code is doing. This is completely missing. For example showing the sections in the DLL.
This is a "normal" 64 bit DLL from Visual Studio 2010:

Section 0: 0000000180001000 ... 00000001800640A2  VirtSize: 000630A3  RawData: 00063200  ".text"
Section 1: 0000000180065000 ... 00000001800798D9  VirtSize: 000148DA  RawData: 00014A00  ".rdata"
Section 2: 000000018007A000 ... 000000018007E238  VirtSize: 00004239  RawData: 00001A00  ".data"
Section 3: 000000018007F000 ... 00000001800821A3  VirtSize: 000031A4  RawData: 00003200  ".pdata"
Section 4: 0000000180083000 ... 000000018008405A  VirtSize: 0000105B  RawData: 00001200  ".idata"
Section 5: 0000000180085000 ... 000000018008590B  VirtSize: 0000090C  RawData: 00000A00  ".rsrc"
Section 6: 0000000180086000 ... 00000001800869FC  VirtSize: 000009FD  RawData: 00000A00  ".reloc"

I don't know why VisualStudio stores these rare values for the virtual size?
All sections are rounded up to the page size of 4096 = 0x1000 bytes.

This is a 32 bit DLL protected with WinLicense:

Section 0: 10001000 ... 1008BFFF  VirtSize: 0008B000  RawData: 0001C000  "   "
Section 1: 1008C000 ... 1008C72B  VirtSize: 0000072C  RawData: 00000800  ".rsrc"
Section 2: 1008D000 ... 1008DFFF  VirtSize: 00001000  RawData: 00000200  ".idata  "
Section 3: 1008E000 ... 104F8FFF  VirtSize: 0046B000  RawData: 00000200  "        "
Section 4: 104F9000 ... 107E5FFF  VirtSize: 002ED000  RawData: 002ECC00  "khputwap"
Section 5: 107E6000 ... 107E6FFF  VirtSize: 00001000  RawData: 00000200  "fdaeohie"

As you see Section 3 has a virtual size of 0x46B000 bytes but contains only 0x200 bytes of code which is copied from the file.
The error of Joachim Bauch was to set the Execute flag only for the first page of 0x1000 byes.
All the rest of the section stayed without execute permission.
With DEP enabled ---> Crash

Section 0: Set Flags: RWE  "   "
Section 1: Set Flags: RW-  ".rsrc"
Section 2: Set Flags: RW-  ".idata  "
Section 3: Set Flags: RWE  "        "
Section 4: Set Flags: RWE  "khputwap"
Section 5: Set Flags: RWE  "fdaeohie"

R = Read, W = Write, E = Execute

[BigJim]

Thanks for your patches and insights on this repo.
Would you make your own fork with your fixes and modifications?

I went with this repo to have a starting point myself because it was the most basic to start with (just one header and C files) and not someone's fanciful C++ machination (most so hard to read!). But making heavy modifications to fit my need (using a preallocated virtual memory section, and then plan on eventually adding support for sections it doesn't currently support (like SEH).

Depends on what the developer wants. I wanted something that is more like shell code with my own minimal DLLs and don't expect it to support all possible DLL usages like the OS does.

Hum.. I think one if they wanted to make the investment, they could go through leaked Windows sources like the WRK, Win2K3, etc., and use the ReactOS open-source too. Walk all the way from ntdll:LdrLoadDll() to kernel and back, filling in the blanks by reverse engineering ntdll.dll and ntoskrnl.exe as needed.

One robust derivative solution that appears to be recent and maintained is this one.
It has exceptions and TLS support, etc.
But it's style of loading up tons of local headers and extra support code is not for me:
https://github.com/bb107/MemoryModulePP

[Elmue]

Hello BigJim

I'am not working anymore on this.
I don't have plans to make yet another fork of this project which already has lots of forks.
If you like, you can do that.

I got quite disappointed on the way because there are too many Microsoft secrets which would have to know to make a really good working own loader.

On XP my modfied code works quite well, although not all is working neither.
But the higher you go in the operating system sup, the worse it becomes.
If we cannot even start Windows Calulator, we are far from having a really working loader.

The problem is simple:
The real Microsoft loader writes a lot of data into their own Kernel memory with informations about the loaded modules.
If you load a DLL from memory and these informations are missing several features will not work correctly.
And even if we knew what to write into these internal lists, how do you get the address of Kernels internal lists?
And tomorrow Microsoft can change the data format and you are pissed.

I think one if they wanted to make the investment, they could go through leaked
Windows sources like the WRK, Win2K3, etc., and use the ReactOS open-source too.

I studied ReactOS very thoroughly.
But this is useless. It is very very old code.
Windows 10 is completely different.
As long as you don't have a Windows 10 leak you will not be able to write a loader which works correctly on Windows 10.
They have changed all the structures and added new fields which are undocumented.

The only way would be a complete reverse engineering of the Kernel.
But who will invest that time ?
Maybe the NSA has that knowledge.
But an ordinary programmer does not live long enough to do that.

It is by purpose that we cannot load a DLL from memory easily.
That is what trojans do.
And it may even become more difficult in the future.