frida / frida-gum

Cross-platform instrumentation and introspection library written in C
https://frida.re
Other
761 stars 248 forks source link

gum modifies TEB!LastErrorValue while running (side effect) #405

Open zuypt opened 4 years ago

zuypt commented 4 years ago

Look at this call stack

0:000> k
 # ChildEBP RetAddr  
00 0117dccc 5e94f689 KERNELBASE!TlsGetValue+0x16
01 0117dcdc 5e96089e frida_agent_32!g_private_get+0x19 [c:\src\frida\glib\glib\gthread-win32.c @ 408] 
02 0117dcf0 5e95f1b3 frida_agent_32!thread_memory_from_self+0x1e [c:\src\frida\glib\glib\gslice.c @ 498] 
03 0117dd18 5e9727f9 frida_agent_32!g_slice_free_chain_with_offset+0x53 [c:\src\frida\glib\glib\gslice.c @ 1183] 
04 0117dd2c 5eb9dec0 frida_agent_32!g_slist_free+0x19 [c:\src\frida\glib\glib\gslist.c @ 137] 
05 0117dec4 5eb978b1 frida_agent_32!_gum_duk_args_parse+0xe20 [d:\frida\frida-gum\bindings\gumjs\gumdukvalue.c @ 461] 
06 0117dee8 5eb8fb1a frida_agent_32!gumjs_native_pointer_construct_impl+0x51 [d:\frida\frida-gum\bindings\gumjs\gumdukcore.c @ 2189] 
07 0117df0c 5eb3005b frida_agent_32!gumjs_native_pointer_construct+0x6a [d:\frida\frida-gum\bindings\gumjs\gumdukcore.c @ 2178] 
08 0117dfb4 5eb163fc frida_agent_32!duk__handle_call_raw+0x3bb [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64373] 
09 0117dfc8 5eaed1ca frida_agent_32!duk_handle_call_unprotected+0x1c [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64527] 
0a 0117dfe0 5eb9f767 frida_agent_32!duk_new+0xaa [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 14457] 
0b 0117dff4 5ebb2a04 frida_agent_32!_gum_duk_push_native_pointer+0xa7 [d:\frida\frida-gum\bindings\gumjs\gumdukvalue.c @ 1316] 
0c 0117e00c 5ebb1bba frida_agent_32!gumjs_instruction_get_address_impl+0x34 [d:\frida\frida-gum\bindings\gumjs\gumdukinstruction.c @ 275] 
0d 0117e030 5eb3005b frida_agent_32!gumjs_instruction_get_address+0x6a [d:\frida\frida-gum\bindings\gumjs\gumdukinstruction.c @ 271] 
0e 0117e0d8 5eb163fc frida_agent_32!duk__handle_call_raw+0x3bb [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64373] 
0f 0117e0ec 5eaecdad frida_agent_32!duk_handle_call_unprotected+0x1c [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64527] 
10 0117e108 5eb07a60 frida_agent_32!duk_call_method+0x9d [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 14269] 
11 0117e1e4 5eb403e2 frida_agent_32!duk_hobject_getprop+0x850 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 56939] 
12 0117f008 5eb168d7 frida_agent_32!duk__js_execute_bytecode_inner+0x5ae2 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 77341] 
13 0117f07c 5eb2ffe6 frida_agent_32!duk_js_execute_bytecode+0xa7 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 76051] 
14 0117f124 5eb163fc frida_agent_32!duk__handle_call_raw+0x346 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64345] 
15 0117f138 5eb1a7d3 frida_agent_32!duk_handle_call_unprotected+0x1c [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64527] 
16 0117f158 5eb304cc frida_agent_32!duk__pcall_raw+0xb3 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 14307] 
17 0117f170 5eb16560 frida_agent_32!duk__handle_safe_call_inner+0x9c [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64580] 
18 0117f208 5eaed373 frida_agent_32!duk_handle_safe_call+0x100 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 64825] 
19 0117f228 5eaecf38 frida_agent_32!duk_safe_call+0xd3 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 14437] 
1a 0117f254 5eb8cd33 frida_agent_32!duk_pcall+0xb8 [d:\frida\frida-gum\bindings\gumjs\duktape.c @ 14326] 
1b 0117f270 5ebd6228 frida_agent_32!_gum_duk_scope_call+0x33 [d:\frida\frida-gum\bindings\gumjs\gumdukcore.c @ 1241] 
1c 0117f364 5ece5bd4 frida_agent_32!gum_duk_callback_transformer_transform_block+0xc8 [d:\frida\frida-gum\bindings\gumjs\gumdukstalker.c @ 774] 
1d 0117f3fc 5ece58b3 frida_agent_32!gum_exec_ctx_obtain_block_for+0x1c4 [d:\frida\frida-gum\gum\backend-x86\gumstalker-x86.c @ 1566] 
1e 0117f418 5eceabbb frida_agent_32!gum_exec_ctx_replace_current_block_with+0x103 [d:\frida\frida-gum\gum\backend-x86\gumstalker-x86.c @ 1466] 
1f 0117f428 03b89126 frida_agent_32!gum_exec_ctx_replace_current_block_from_jmp_cond_imm+0x3b [d:\frida\frida-gum\gum\backend-x86\gumstalker-x86.c @ 1425] 
WARNING: Frame IP not in any known module. Following frames may be wrong.
20 0117f82c 00801058 0x3b89126
21 0117f848 0080131f test_bin+0x1058
22 0117f890 77846359 test_bin+0x131f
23 0117f8a0 77cd7b74 KERNEL32!BaseThreadInitThunk+0x19
24 0117f8fc 77cd7b44 ntdll!__RtlUserThreadStart+0x2f
25 0117f90c 00000000 ntdll!_RtlUserThreadStart+0x1b

g_private_get calls TlsAlloc which calls TlsGetValue which ends up modifies LastErrorValue. Many API functions (in this case gumjs_instruction_get_address) end up in g_private_get. This easily messes up code that uses GetLastError for error handling.

POC

#include <Windows.h>
#include <stdio.h>

int main(int argc, char** argv) {
    SetLastError(0x1337);
    printf("GetLastError: %x\n", GetLastError());
import frida

t = '''
function initStalker()
{
    Stalker.trustThreshold = 0;
    var t = Process.enumerateThreads()[0]
    Stalker.follow(t.id,
    {
        transform: function(iterator)
        {
            var lolol = new NativePointer(123)
            while (iterator.next() != null) iterator.keep();
        }
    })
}

initStalker()
'''
pid = frida.spawn(['test_bin.exe'])
session = frida.attach(pid)
script = session.create_script(t)
script.load()
frida.resume(pid)
sys.stdin.read()

expected ouput GetLastError: 1337

result output GetLastError: 0

I don't see any straight forward way to fix this issue. Give me some hints then i maybe I can fix it and make a pull request.

oleavr commented 4 years ago

Oh, nice find! I suppose we can query it first, and restore it afterwards?

zuypt commented 4 years ago

Oh, nice find! I suppose we can query it first, and restore it afterwards?

I don't really know how frida is structured .Do I need to fork pkg-config or glib repo to fix this issue ?

oleavr commented 4 years ago

We could definitely improve g_private_get() so it doesn't clobber LastError, but I think this is a deeper issue with transformers. The Stalker integration should probably save and restore it before/after returning, just like Interceptor does for onEnter / onLeave.