Modifying program using IDA/Hexrays decompilation pseudocode

Question

A bit long-winded, apologies in advance. Updates to the question are at the end.

I'm experimenting with hooking functions in a program & modifying them based on the decompilation pseudocode from IDA. For this, I wrote a simple, purposeless, 32-bit program (it's not important, no need to try and analyze it):

// main.cpp
#include <iostream>


class C {
    public:
    int a = 0;
    int b = 0;
    int c[255];
    int *d = nullptr;
    int stuff = 0;
    C(){
        std::cout << d << std::endl;
        d = new int[300];
        std::cout << d << std::endl;
        ++stuff;
        for(int i = 1; i < 255; ++i){
            c[i] = i;
            if( i == 254){
                c[i] = 0;
            }
        }
        for(int i = 1; i < 300; ++i){
            d[i] = i;
            if( i == 299){
                d[i] = 0;
            }
        }
    }
    ~C(){
        std::cout << d << std::endl;
        delete[] d;
        std::cout << d << std::endl;
        ++stuff;
    }

    void play(){
        bool c_stop, d_stop;
        c_stop = false;
        d_stop = false;
        for(int i = 0;;++i){
            if(c[i] == 0 && !c_stop){
                std::cout << "C had " << i + 1<< std::endl;
                c_stop = true;
            }
            if(d[i] == 0 && !d_stop){
                std::cout << "D had " << i + 1 << std::endl;
                d_stop = true;
            }
            if(d_stop && c_stop){
                break;
            }
        }
    }
};

void func(){
    C clae;
    std::cout << "One missisipi " << clae.stuff << std::endl;
}

int main(){
    func();
    C clae;
    std::cout << clae.stuff << std::endl;
    clae.play();
}

While there's no need to try and understand what it does, it will usually result in an output like this:

00000000
0007D958
One missisipi 1
0007D958
0007D958
00000000
0007D958
1
C had 255
D had 300
0007D958
0007D958

I say usually because the addresses will change from execution to execution.

Now, using IDA, I found the subroutine corresponding to void func():

void *__thiscall sub_1161000(void *this)
{
  int v1; // eax@1
  int v2; // edx@1
  int v4; // [sp+0h] [bp-43Ch]@1
  int v5; // [sp+8h] [bp-434h]@1
  int v6; // [sp+Ch] [bp-430h]@1
  int v7; // [sp+10h] [bp-42Ch]@1
  char v8; // [sp+14h] [bp-428h]@1
  int v9; // [sp+41Ch] [bp-20h]@1
  int *v10; // [sp+420h] [bp-1Ch]@1
  void *v11; // [sp+424h] [bp-18h]@1
  int (__cdecl *v12)(int, int, int, int); // [sp+428h] [bp-14h]@1
  int v13; // [sp+42Ch] [bp-10h]@1

  v10 = &v4;
  v13 = -1;
  v12 = sub_1161200;
  v11 = this;
  v1 = sub_1161280(&v8);
  v13 = 0;
  v7 = v1;
  v6 = sub_11613F0(&unk_119B300, "One missisipi ");
  v5 = sub_1161810(v9);
  sub_1161AB0(sub_1161AD0);
  sub_1161B30((int)&v8, v2);
  return v11;
}

I am certain this is the function at hand, because stepping over the call to this address prints:

00000000
0007D958
One missisipi 1
0007D958
0007D958

Which is what is expected of func() to output. Furthermore, stepping inside this subroutine will print the above but in stages corresponding to the constructor, print, and destructor.

So, now I have patched the pseudocode into valid C, like so:

void *__thiscall func(void *_this) {
    int v1;                                 // eax@1
    int v2;                                 // edx@1
    int v4;                                 // [sp+0h] [bp-43Ch]@1
    int v5;                                 // [sp+8h] [bp-434h]@1
    int v6;                                 // [sp+Ch] [bp-430h]@1
    int v7;                                 // [sp+10h] [bp-42Ch]@1
    char v8;                                // [sp+14h] [bp-428h]@1
    int v9;                                 // [sp+41Ch] [bp-20h]@1
    int *v10;                               // [sp+420h] [bp-1Ch]@1
    void *v11;                              // [sp+424h] [bp-18h]@1
    int(__cdecl * v12)(int, int, int, int); // [sp+428h] [bp-14h]@1
    int v13;                                // [sp+42Ch] [bp-10h]@1

    int *base_addr = (int *)GetModuleHandle(NULL); // Windows function to get the process base address.

    v10 = &v4;
    v13 = -1;
    v12 = INT_FUNC(0x1161200);
    v11 = _this;
    v1 = INT_FUNC(0x1161280)(&v8);
    v13 = 0;
    v7 = v1;
    v6 = INT_FUNC(0x11613F0)(0x119B300, "One missisipi ");
    v5 = INT_FUNC(0x1161810)(v9);
    PTR_FUNC(0x1161AB0)(PTR_FUNC(0x1161AD0));
    PTR_FUNC(0x1161B30)((int)&v8, v2);
    return v11;
}

Where INT_FUNC is defined as:

#define PE_BASE 0x1160000

#define OFFSET(x) (int*)(base_addr + ((int*)x - (int*)PE_BASE))

#define PTR_FUNC(x) (((void *(*)())OFFSET(x)))
#define INT_FUNC(x) (((int (*)())OFFSET(x)))

What I did was some pointer arithmetic to convert the addresses from IDA to addresses which would correspond to the proper ones during runtime, and cast them to a function. PE_BASE is the base address of the process in IDA (I know you can rebase them to zero). Note that at this stage, I did not modify the function at all, but only recreated it from the pseudocode.

Finally, to hook my function, I use a debugger to start the program, load a DLL containing my custom func, and then at the start of the target subroutine, I replaced the instructions there with:

push dword my_dll.func
ret

Which happily jumps to my function whenever the target subroutine is being executed. The thing is though, it just doesn't work. If I run my program now, it only prints:

00000000
01393FE8

I debugged the program while my function was hooked, and the culprit was this line, which causes a segmentation fault:

Which is the first function address that is being called this way (the function before it is not called, but only pass its address to v12). My first thought was that a mistake in my pointer arithmetic caused it to jump to a bad address, but checking the disassembly confirmed that this is not the case:

Which is the correct function (based on value of edx in the previous picture):

It appears everything is where it should be.

So, my question is: What's going on? Why does it do this? Is my attempt at using IDA's pseudocode a lost cause because it cannot be trusted to be reliable? Or, is there some mistake on my part?

UPDATE:

Thanks to bart1e's answer, I now fixed the code to reflect the actual size of the variables as commented by IDA (Code is more simplified since the original version, but the issues are still the same):

int func() {
    char v1[0x408]; // [sp+18h] [bp-414h]@1
    int v2;  // [sp+420h] [bp-Ch]@1

    int* base_addr = (int*)GetModuleHandle(NULL);

    INT_FUNC(0x331F04)(v1);
    INT_FUNC(0x3B7E90)(OFFSET(0x3C57C0), "One missisipi ");
    INT_FUNC(0x37E7C0)(v2); // <<<<<<<< SEGMENTATION FAULT
    INT_FUNC(0x37E5C0)(INT_FUNC(0x3B5FD0));
    return INT_FUNC(0x332044)(v1);
}

/* int func()
{
  char v1; // [sp+18h] [bp-414h]@1
  int v2; // [sp+420h] [bp-Ch]@1

  sub_331F04((int)&v1);
  sub_3B7E90((int)&dword_3C57C0, "One missisipi ");
  sub_37E7C0(v2);
  sub_37E5C0(sub_3B5FD0);
  return sub_332044(&v1);
} */

Now however, when I try to debug the DLL, it still results in a segmentation fault, and print:

00000000
015CFFE8
One missisipi

Answer 1

If you compare pseudocode returned by decompiler with your own C code you will notice one very important difference - in decompiler's output it is merely a comment, but it's very important. I'm talking about following lines:

int v7;                                 // [sp+10h] [bp-42Ch]@1
char v8;                                // [sp+14h] [bp-428h]@1
int v9;                                 // [sp+41Ch] [bp-20h]@1

Look where variables v7, v8 and v9 are located on the stack:

• v7 is located at sp + 0x10 and requires 4 bytes, which is fine - just what int needs
• v8 is char but requires 0x41C - 0x14 = 0x408 bytes. Something seems wrong here; if you declare char in C/C++, compiler will allocate just 1 byte, definitely not 0x408!

What happens here is, v8 represents space allocated on stack to hold your clae (class C) object and each such object occupies 0x408 bytes (possibly even more; disassembly line mov dword ptr [eax+408h], 0 of constructor suggests it occupies at least v9's space as well).

But decompiler doesn't know this and what it sees is that there is some function sub_1161280 (constructor in this case) and it takes some address, which is pointer to a stack memory (in fact, pointer to a newly created object).

Decompiler doesn't have an idea that this pointer actually points to a blob of 0x408 bytes - it sees that just byte at v8 is referenced and isn't sure what other 0x407 bytes are used for; thats why it creates just 1-byte variable v8 and doesn't name remaining 0x407 bytes. It could be char v8[0x408], but as well char v8 and then just any data consistng of 0x407 bytes (some ints maybe, then possibly other chars, etc.).

So, if you change char v8 to char v8[0x408] it should be fine, but I didn't analyse your code so thoroughly to be sure there aren't any more mistakes.

And, by the way, decompilers often produce inaccurate, unreadable or even incorrect results, so you should never fully rely on them; they can be helpful in some cases, but confusing as well. And what they produce is merely a pseudocode which is just a hint on how original function might have looked like, but definitely not fully working code.