0

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:

enter image description here

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:

enter image description here

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

enter image description here enter image description here

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
4
  • Use structures - I've personally just written a very very basic tool that simply copies the frame structure and puts a single variable with that new structure in the beginning of the stack frame - then when you use Hex-Rays it will still occasionally create separate variables but mostly tolerate your structure.
    – rec
    Mar 10 at 16:50
  • Because structures most closely represent what's happening in asm. A stack is a structure not some random variables scattered around.
    – rec
    Mar 10 at 16:52
  • @rec It's funny, right before you said that, I decided to remove the base_addr variable and place it in a structure containing the modified function. I still haven't tested this however. Is this what you mean by using structures? If not, could you elaborate further?
    – daedsidog
    Mar 10 at 17:00
  • I mean right from ida - recreate your stack frame containing a single variable (at the beginning) of structure type containing all the locals. Then you can run hex-rays again. But I don't know if that will help your specific case - however it's one good beginning in the general one.
    – rec
    Mar 10 at 17:37
3
+50

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.

6
  • I never would have thought of that, but it makes perfect sense! However, after updating the pseudocode to reflect your insight, it still results in a segmentation fault on the same line, only this time, it actually manages to print "One missisipi".
    – daedsidog
    Mar 6 at 17:07
  • Yes, as I wrote, there are possibly more mistakes in decompiled code than this one. If you uploaded your files somewhere and gave a link to them it would be easier to spot other bugs.
    – bart1e
    Mar 6 at 17:09
  • As per your request, I created a repository for this question: github.com/daedsidog/ida-pseudocode-recompilation-win32
    – daedsidog
    Mar 6 at 18:05
  • @daedsidog It seems each object of your class consists of 0x418 bytes (v9 from first pseudocode is a sfuff variable, v10 is constructor ptr, v11 - destructor ptr and v12 is ptr to play()), so you cannot just get rid of these variables since otherwise return address after a call to constructor will be overwritten by some random data causing segfault. Try changing char v1[0x408] to char v1[0x418] and get rid of v2. That should do the job. I will update my answer if it helps. Didn’t have time to test it yet.
    – bart1e
    Mar 7 at 23:04
  • I assume you mean for me to do that in the updated pseudocode? v2 is being used by one of the functions later on, but even if I did what you said, it behaves the same way. The new pseudocode is what IDA gave me after I recompiled the program statically.
    – daedsidog
    Mar 8 at 4:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.