1

First thing first:

My Test Systems: Windows 7 32 bit Professional Windows 7 64 bit Professional

Compiler: gcc version 5.2.0 (i686-win32-dwarf-rev0, Built by MinGW-W64 project)

Build options: g++ foo.cpp -o foo.exe

My application receives function addresses from the kernel32.dll dynamically. This works as intended. However, when I xor encrypt those API calls and pass the decrypted string to the GetProcAdress function, it failes depending on the order of char pVAE_D[strlen(pVAE_E)]; and char pGTC_D[strlen(pGTC_E)] as well as the order of the encrypted strings; If I set char pGTC_D[strlen(pGTC_E)] before char pVAE_D[strlen(pVAE_E)] (same for the encrypted strings)

`#include <windows.h>
#include <stdio.h>

typedef BOOL(WINAPI* _GetThreadContext)
(HANDLE hThread, 
LPCONTEXT lpContext
); 

typedef LPVOID(WINAPI* _VirtualAllocEx)
(HANDLE handle_Process, 
LPVOID longpointer_Address, 
SIZE_T dwSize, 
DWORD flAllocationType, 
DWORD flProtect
);

void encrStr(char *pStr, int len, char *pOut, char cryptochar);

int main(void)
{   
char pVAE_E[] = {0x32, 0x0d, 0x16, 0x10, 0x11, 0x05, 0x08, 0x25, 0x08, 0x08,   

0x0b, 0x07, 0x21, 0x1c, 0x00};  // VirtualAllocEx encrypted with d

char pGTC_E[] = {
0x3d, 0x1f, 0x0e, 0x2e, 0x12, 0x08, 0x1f, 0x1b, 0x1e, 0x39, 0x15, 0x14, 
0x0e, 0x1f, 0x02, 0x0e, 0x00};  // GetThreadContext encrypted with z

char pVAE_D[strlen(pVAE_E)]; 
char pGTC_D[strlen(pGTC_E)];


encrStr(pVAE_E, strlen(pVAE_E), pVAE_D, 'd');
encrStr(pGTC_E, strlen(pGTC_E), pGTC_D, 'z');
HMODULE hKernel32 = LoadLibraryA("kernel32.dll");

FARPROC fpGetThreadContext = GetProcAddress(hKernel32, pGTC_D); 
if (fpGetThreadContext == NULL)
{
    printf("gtc failed.\n"); 
}

_GetThreadContext kernel32GetThreadContext =      

(_GetThreadContext)fpGetThreadContext; 

FARPROC fpVirtualAllocEx = GetProcAddress(hKernel32, pVAE_D); 
if (fpVirtualAllocEx == NULL)
{
    printf("vae failed.\n"); 
}
_VirtualAllocEx kernel32VirtualAllocEx = (_VirtualAllocEx)fpVirtualAllocEx; 


}

void encrStr(char *pStr, int len, char *pOut, char cryptochar)
{
// zero char must remain, therefore i < len
for (int i = 0; i < len; i++)
{
    pOut[i] = pStr[i] ^ cryptochar; 
    printf("%c\n", pOut[i]); 
}
pOut[len] = 0x00; 
printf("%s\n", pOut); 
}`

If I increase the memory size of the decryption buffer by 1 it works independent from the assignment order. Why does it fail in the otherwise ?

lambda
  • 69
  • 5
  • Please strip unrelated information and code from your question, and concentrate on the issue you have. Do provide an [MCVE](http://stackoverflow.com/help/mcve) (emphasis on *minimal*). Calls to `LoadLibraryA`, `GetProcAddress` and the related function pointer invocations are not related to the issue at all. – IInspectable Jan 15 '16 at 19:46
  • 1
    usually `pOut[len]` is invalid index. The last element should probably be `pOut[len - 1]` – Barmak Shemirani Jan 15 '16 at 20:46

1 Answers1

1

The are two problems with your code. First is that you are using strlen to determine the size of the buffers for decrypted strings. This function does not take into account the null-terminator character '\0' (which is 0x00 in your case). This means that buffers pVAE_D and pGTC_D will have one element less than their corresponding buffers containing encrypted strings. The second problem is your encrStr function. In it, you are writing beyond the bounds of the array with statement pOut[len] = 0x00;. Indices for accessing the array are in range [0, len - 1]. This is how your function (assuming that you are passing an output buffer with enough elements for string and its null-terminator) should be implemented:

void encrStr(char *pStr, int len, char *pOut, char cryptochar)
{
    // zero char must remain, therefore i < len
    for (int i = 0; i < len - 1; i++)
    {
        pOut[i] = pStr[i] ^ cryptochar; 
        printf("%c\n", pOut[i]); 
    }
    pOut[len - 1] = 0x00; 
    printf("%s\n", pOut); 
}

When you write beyond bounds of the array, you are corrupting the stack around your array and you encounter UB (undefined behavior). That is why this sample sometimes works, and some times it does not. Some compilers will warn you about this, but others won't. These are subtle bugs in C++ that can cause you a lot of headache. Checkout these links for more info about undefined behavior:

What are all the common undefined behaviours that a C++ programmer should know about?

https://en.wikipedia.org/wiki/Undefined_behavior

Community
  • 1
  • 1
Marko Popovic
  • 3,999
  • 3
  • 22
  • 37
  • Thanks, that pretty much solved the issue, I understand why it failed now. I recompiled it with your improved code sample, it works properly. – lambda Jan 16 '16 at 14:23