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

NativeCallback doesn't seem to work on Windows, except for mscdecl #525

Closed
taviso opened this issue Nov 19, 2019 · 9 comments
Closed

NativeCallback doesn't seem to work on Windows, except for mscdecl #525

taviso opened this issue Nov 19, 2019 · 9 comments

Comments

@taviso
Copy link

taviso commented Nov 19, 2019

I can't seem to get NativeCallback to work on Windows/x86 with anything other than mscdecl. The stack is not being set up or cleared correctly for the other abis, but I'm not sure if I'm using it wrong, should these examples work?

(I'm just typing these into devtools, but I've tried using it from a script and also can't get it to work)

var cb = new NativeCallback(function () { console.log("hello"); }, "void", [], "mscdecl");
var func = new NativeFunction(cb, "void", [], "mscdecl");
func() // This works
var cb = new NativeCallback(function () { console.log("hello"); }, "void", [], "stdcall");
var func = new NativeFunction(cb, "void", [], "stdcall");
func() // Error: access violation
var cb = new NativeCallback(function () { console.log("hello"); }, "void", [], "thiscall");
var func = new NativeFunction(cb, "void", [], "thiscall");
func() // Error: access violation
var cb = new NativeCallback(function () { console.log("hello"); }, "void", [], "fastcall");
var func = new NativeFunction(cb, "void", [], "fastcall");
func() // Error: access violation

I think there's a documentation bug where it says "You may also specify the abi if not system default", but it seems to assume mscdecl which is not the system default on Windows, and doesn't match what NativeFunction assumes is the system default (stdcall).

@taviso
Copy link
Author

taviso commented Nov 19, 2019

Here is a temporary workaround for anyone else needing other abis, you can make calling convention wrapper generators.

Here is my one for thiscall:

function genThiscallWrapper(param: NativeCallback, numArgs: number) {
    var impl = Memory.alloc(Process.pageSize);
    Memory.patchCode(impl, 64, function (code: NativePointer) {
        var cw = new X86Writer(code, { pc: impl });

        // Subtract one argument, because it's passed via register.
        numArgs--;

        // push dword [esp+n]
        for (let i = 0; i < numArgs; i++) {
            cw.putBytes(new Uint8Array([0xFF, 0x74, 0x24, 4 * numArgs]))
        }
        cw.putPushReg("ecx"); // this
        cw.putCallAddress(param);
        cw.putAddRegImm("esp", 4 + 4 * numArgs);
        cw.putRetImm(4 * numArgs);
        cw.flush();
    });
    return impl;
};

You use it like so, if you wanted to write this:

let handler = new NativeCallback(callback, "void", ["pointer", "int", "pointer"], "thiscall");

Don't do that, because it will crash. Instead, write this:

// The '3' is the total number of parameters.
let callback = new NativeCallback(callback, "void", ["pointer", "int", "pointer"]);
let handler = genThiscallWrapper(callback, 3);

And it will work.

@Meigyoku-Thmn
Copy link

Meigyoku-Thmn commented Jun 26, 2020

Was this bug fixed? I also encountered the same problem. Only cdecl calling convention works on NativeCallback.

@Meigyoku-Thmn
Copy link

Meigyoku-Thmn commented Dec 7, 2020

There was a mistake in @taviso's solution: the loop that pushes the arguments also changes the esp register, resulting corrupted arguments.
I have another solution for stdcall (32-bit) that works correctly for any number of passed arguments (not greater than 127 because nobody pass that many), for thiscall, just subtract one from numArgs and push the ecx along:

export function wrapCdecl2Stdcall(func: NativeCallback, numArgs: number): NativePointer {
   if (numArgs > 127 || numArgs < 0)
      throw new Error('numArgs is out of range [0, 127]');
   const wrapperFunc = Memory.alloc(Process.pageSize);
   Memory.patchCode(wrapperFunc, Process.pageSize, code => {
      const cw = new X86Writer(code, { pc: wrapperFunc });
      cw.putPushReg('ebp');                // push   ebp                 -- store the outer stack frame
      cw.putMovRegReg('ebp', 'esp');       // mov    ebp, esp            -- make a stack frame
      cw.putPushReg('ebx');                // push   ebx
      cw.putPushReg('esi');                // push   esi
      cw.putPushReg('edi');                // push   edi                 -- preserve ebx, esi and edi     
      for (let i = numArgs + 1; i > 1; i--) {
         cw.putBytes(new Uint8Array([0xff, 0x75, 4 * i]).buffer as ArrayBuffer);
      }                                    // push   DWORD PTR [ebp + i] -- push arguments for cdecl call
      cw.putCallAddress(func);             // call   func                -- call the cdecl function
      cw.putAddRegImm('esp', 4 * numArgs); // add    esp, 4 * numArg     -- clear the pushed arguments
      cw.putPopReg('edi');                 // pop    edi
      cw.putPopReg('esi');                 // pop    esi
      cw.putPopReg('ebx');                 // pop    ebx                 -- restore ebx, esi and edi
      cw.putMovRegReg('esp', 'ebp');       // mov    esp, ebp            -- remove the stack frame (also clear every local vars)
      cw.putPopReg('ebp');                 // pop    ebp                 -- restore the outer stack frame
      cw.putRetImm(4 * numArgs);           // ret    4 * numArgs         -- return and clear all arguments
      cw.flush();
   });
   return wrapperFunc;
}

Then you can use it like this (the KeyboardProc callback)

const KeyboardProc = new NativeCallback((code: number, wParam: number, lParam: number) => {
    if (code >= 0) 
        console.log(JSON.stringify({code, wParam, lParam}));
    return CallNextHookEx(0, code, wParam, lParam);
}, 'int32', ['int32', 'uint32', 'int32'], 'mscdecl');
const KeyboardProcWrapper = wrapCdecl2Stdcall(KeyboardProc, 3);

The wrapper function must preserve the ESI, EDI, EBX and EBP registers if you use it as a callback for certain WinAPIs.

@taviso
Copy link
Author

taviso commented Dec 7, 2020

I don't think it's a mistake (I might be wrong, but it definitely works when testing!) - The idea is to duplicate the stdcall parameters, but because pushing one argument also moves the pointer the next argument gets aligned for free.

The reason I didn't save/preserve other registers is because (as you said) the callee must preserve them, so as long as I don't modify them then there's no need to replace them.

@Meigyoku-Thmn
Copy link

I don't think it's a mistake (I might be wrong, but it definitely works when testing!) - The idea is to duplicate the stdcall parameters, but because pushing one argument also moves the pointer the next argument gets aligned for free.

The reason I didn't save/preserve other registers is because (as you said) the callee must preserve them, so as long as I don't modify them then there's no need to replace them.

Ah sorry, it's actually my mistake, in my code I actually use the "i" index to push the arguments. There are a lot of thing going on in my code so I thought there was something wrong in your code.

@taviso
Copy link
Author

taviso commented Dec 7, 2020

Ah, I see what you mean - I really should have made a comment explaining that!

@oleavr
Copy link
Member

oleavr commented Dec 7, 2020

@taviso I just updated all of our dependencies in 14.1, so I'm curious if this is still broken? If so it's a bug in libffi upstream, or the Meson build system that we maintain on top of it. (Perhaps we're not configuring it correctly, so the additional ABIs are unavailable.)

@taviso
Copy link
Author

taviso commented Dec 8, 2020

Yes, I just checked, it still happens in 14.1

@oleavr oleavr transferred this issue from frida/frida Jan 4, 2021
oleavr added a commit that referenced this issue Jan 4, 2021
These now pass with our recently rebased and improved libffi.

Related to #525.
@oleavr
Copy link
Member

oleavr commented Jan 7, 2021

This is now finally fixed in Frida 14.2.3, and libffi patches have been submitted upstream 🎉 Thanks a lot for reporting and helping narrow this one down!

@oleavr oleavr closed this as completed Jan 7, 2021
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

3 participants