From 48a142f8aebb68e501659b68596fc6099926388e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ole=20Andr=C3=A9=20Vadla=20Ravn=C3=A5s?= Date: Mon, 13 May 2024 16:12:54 +0200 Subject: [PATCH] Fix minor nits --- lib/payload/meson.build | 4 +- lib/payload/unwind-sitter-glue.c | 98 +++++++++++++------------------ lib/payload/unwind-sitter.vala | 5 +- tests/labrats/exception-catcher.m | 23 ++++---- tests/test-host-session.vala | 20 +++++-- 5 files changed, 69 insertions(+), 81 deletions(-) diff --git a/lib/payload/meson.build b/lib/payload/meson.build index ad320c144..944ff9534 100644 --- a/lib/payload/meson.build +++ b/lib/payload/meson.build @@ -9,13 +9,13 @@ payload_sources = [ 'spawn-monitor-glue.c', 'thread-suspend-monitor.vala', 'thread-suspend-monitor-glue.c', + 'unwind-sitter.vala', + 'unwind-sitter-glue.c', 'exit-monitor.vala', 'cloak.vala', 'fd-guard.vala', 'fdt-padder.vala', 'libc-shim.c', - 'unwind-sitter.vala', - 'unwind-sitter-glue.c' ] if host_os_family == 'linux' diff --git a/lib/payload/unwind-sitter-glue.c b/lib/payload/unwind-sitter-glue.c index cfd81ce89..a596262f3 100644 --- a/lib/payload/unwind-sitter-glue.c +++ b/lib/payload/unwind-sitter-glue.c @@ -93,16 +93,13 @@ _frida_unwind_sitter_fill_unwind_sections (GumAddress invader_start, GumAddress } void -_frida_unwind_sitter_hook_libunwind () +_frida_unwind_sitter_hook_libunwind (void) { +#if GLIB_SIZEOF_VOID_P == 8 gpointer * set_info_slot; gpointer get_reg_impl; GumInterceptor * interceptor; -#if GLIB_SIZEOF_VOID_P != 8 - return; -#endif - if (state != NULL) return; @@ -112,15 +109,13 @@ _frida_unwind_sitter_hook_libunwind () state->vtable = frida_find_vtable (); if (state->vtable == NULL) - goto beach; + goto unsupported_version; if (!frida_compute_vtable_shift (state->vtable, &state->shift)) - goto beach; + goto unsupported_version; - set_info_slot = (gpointer *)(GUM_ADDRESS (state->vtable) + - UNWIND_CURSOR_VTABLE_OFFSET_SET_INFO + state->shift); - get_reg_impl = *(gpointer *)(GUM_ADDRESS (state->vtable) + - UNWIND_CURSOR_VTABLE_OFFSET_GET_REG + state->shift); + set_info_slot = (gpointer *) (GUM_ADDRESS (state->vtable) + UNWIND_CURSOR_VTABLE_OFFSET_SET_INFO + state->shift); + get_reg_impl = *(gpointer *) (GUM_ADDRESS (state->vtable) + UNWIND_CURSOR_VTABLE_OFFSET_GET_REG + state->shift); state->set_info_slot = set_info_slot; state->set_info_original = *set_info_slot; @@ -128,27 +123,26 @@ _frida_unwind_sitter_hook_libunwind () state->get_reg = RESIGN_PTR (get_reg_impl); interceptor = gum_interceptor_obtain (); - if (gum_interceptor_replace (interceptor, state->set_info_original, frida_unwind_cursor_set_info_replacement, NULL, NULL) != GUM_REPLACE_OK) - goto beach; + if (gum_interceptor_replace (interceptor, state->set_info_original, frida_unwind_cursor_set_info_replacement, NULL, NULL) + != GUM_REPLACE_OK) + goto unsupported_version; return; -beach: +unsupported_version: g_slice_free (UnwindHookState, state); state = NULL; +#endif } void -_frida_unwind_sitter_unhook_libunwind () +_frida_unwind_sitter_unhook_libunwind (void) { GumInterceptor * interceptor; if (state == NULL) return; - if (state->set_info_slot == NULL || state->set_info_original == NULL) - goto beach; - interceptor = gum_interceptor_obtain (); gum_interceptor_revert (interceptor, state->set_info_original); @@ -175,30 +169,27 @@ static DyldUnwindSections * frida_create_cached_sections (CreateArgs * args) { DyldUnwindSections * cached_sections; - GumDarwinModule * module; - FillInfoContext ctx; gsize page_size; gpointer header; GumPageProtection prot; + GumDarwinModule * module; + FillInfoContext ctx; - page_size = getpagesize (); + page_size = gum_query_page_size (); header = GSIZE_TO_POINTER (args->range_start); while ((gum_memory_query_protection (header, &prot) && (prot & GUM_PAGE_READ) == 0) || - (*(guint32 *)header != MH_MAGIC_64 && header + 4 <= GSIZE_TO_POINTER (args->range_end))) + (*(guint32 *) header != MH_MAGIC_64 && header + 4 <= GSIZE_TO_POINTER (args->range_end))) + { header += page_size; - - if (*(guint32 *)header != MH_MAGIC_64) + } + if (*(guint32 *) header != MH_MAGIC_64) return NULL; cached_sections = g_slice_new0 (DyldUnwindSections); - if (cached_sections == NULL) - return NULL; - cached_sections->mh = header; - module = gum_darwin_module_new_from_memory ("Frida", mach_task_self (), GPOINTER_TO_SIZE (header), - GUM_DARWIN_MODULE_FLAGS_NONE, NULL); + module = gum_darwin_module_new_from_memory ("Frida", mach_task_self (), GPOINTER_TO_SIZE (header), GUM_DARWIN_MODULE_FLAGS_NONE, NULL); if (module == NULL) return cached_sections; @@ -253,18 +244,17 @@ frida_unwind_cursor_set_info_replacement (gpointer self, int is_return_address) #else fp = GUM_ADDRESS (state->get_reg (self, UNW_X86_64_RBP)); #endif - if (fp == 0 || fp == -1) return; - missing_info = *((guint8 *)self + UNWIND_CURSOR_unwindInfoMissing); + missing_info = *((guint8 *) self + UNWIND_CURSOR_unwindInfoMissing); stored_pc_slot = GSIZE_TO_POINTER (fp + GLIB_SIZEOF_VOID_P); stored_pc = GUM_ADDRESS (*stored_pc_slot); #if __has_feature (ptrauth_calls) stored_pc = gum_strip_code_address (stored_pc); #elif defined (HAVE_ARM64) - was_signed = (stored_pc & ~STRIP_MASK ) != 0ULL; + was_signed = (stored_pc & ~STRIP_MASK) != 0ULL; if (was_signed) stored_pc &= STRIP_MASK; #endif @@ -273,17 +263,12 @@ frida_unwind_cursor_set_info_replacement (gpointer self, int is_return_address) { GumAddress translated; - translated = GUM_ADDRESS ( - gum_invocation_stack_translate ( - gum_interceptor_get_current_stack (), - GSIZE_TO_POINTER (stored_pc))); - + translated = GUM_ADDRESS (gum_invocation_stack_translate (gum_interceptor_get_current_stack (), GSIZE_TO_POINTER (stored_pc))); if (translated != stored_pc) { #if __has_feature (ptrauth_calls) *stored_pc_slot = ptrauth_sign_unauthenticated ( - ptrauth_strip (GSIZE_TO_POINTER (translated), ptrauth_key_asia), - ptrauth_key_asib, FP_TO_SP (fp)); + ptrauth_strip (GSIZE_TO_POINTER (translated), ptrauth_key_asia), ptrauth_key_asib, FP_TO_SP (fp)); #elif defined (HAVE_ARM64) if (was_signed) { @@ -296,7 +281,7 @@ frida_unwind_cursor_set_info_replacement (gpointer self, int is_return_address) "mov %0, x17\n\t" : "=r" (resigned) : "r" (translated & STRIP_MASK), - "r" (FP_TO_SP(fp)) + "r" (FP_TO_SP (fp)) : "x16", "x17" ); @@ -317,14 +302,14 @@ static gpointer frida_find_vtable (void) { GumAddress result = 0; - csh capstone; + GumAddress export; + uint64_t address; cs_err err; + csh capstone; cs_insn * insn = NULL; - GumAddress export; const uint8_t * code; size_t size; const size_t max_size = 2048; - uint64_t address; export = gum_module_find_export_by_name (LIBUNWIND, "unw_init_local"); if (export == 0) @@ -343,13 +328,9 @@ frida_find_vtable (void) err = cs_open (CS_ARCH_X86, CS_MODE_64, &capstone); #endif g_assert (err == CS_ERR_OK); - if (err != CS_ERR_OK) - goto beach; err = cs_option (capstone, CS_OPT_DETAIL, CS_OPT_ON); g_assert (err == CS_ERR_OK); - if (err != CS_ERR_OK) - goto beach; insn = cs_malloc (capstone); code = GSIZE_TO_POINTER (export); @@ -377,8 +358,12 @@ frida_find_vtable (void) } else if (insn->id == ARM64_INS_ADD && insn->detail->arm64.operands[0].reg == last_adrp_reg) { - GumAddress candidate = last_adrp + (GumAddress) insn->detail->arm64.operands[2].imm; - gboolean is_bss = bss_range.base_address != 0 && + GumAddress candidate; + gboolean is_bss; + + candidate = last_adrp + (GumAddress) insn->detail->arm64.operands[2].imm; + + is_bss = bss_range.base_address != 0 && bss_range.base_address <= candidate && candidate < bss_range.base_address + bss_range.size; if (!is_bss) @@ -406,7 +391,7 @@ frida_find_vtable (void) { if (insn->id == X86_INS_RET) break; - if (insn->id == X86_INS_LEA && insn->detail->x86.op_count == 2) + if (insn->id == X86_INS_LEA) { const cs_x86_op * op = &insn->detail->x86.operands[1]; if (op->type == X86_OP_MEM && op->mem.base == X86_REG_RIP) @@ -445,8 +430,8 @@ static gboolean frida_compute_vtable_shift (gpointer vtable, gssize * shift) { gboolean result = FALSE; - csh capstone; cs_err err; + csh capstone; cs_insn * insn = NULL; const uint8_t * code; uint64_t address; @@ -454,13 +439,10 @@ frida_compute_vtable_shift (gpointer vtable, gssize * shift) cs_arch_register_arm64 (); err = cs_open (CS_ARCH_ARM64, CS_MODE_LITTLE_ENDIAN, &capstone); - g_assert (err == CS_ERR_OK); - if (err != CS_ERR_OK) - goto beach; insn = cs_malloc (capstone); - code = gum_strip_code_pointer (*(gpointer *)vtable); + code = gum_strip_code_pointer (*(gpointer *) vtable); address = GPOINTER_TO_SIZE (code); if (cs_disasm_iter (capstone, &code, &size, &address, insn)) @@ -489,14 +471,14 @@ frida_compute_vtable_shift (gpointer vtable, gssize * shift) GumAddress cursor = GPOINTER_TO_SIZE (vtable); GumAddress error = cursor + 16 * GLIB_SIZEOF_VOID_P; - while (cursor < error && *(gpointer *)GSIZE_TO_POINTER (cursor) == NULL) + while (cursor < error && *(gpointer *) GSIZE_TO_POINTER (cursor) == NULL) cursor += GLIB_SIZEOF_VOID_P; if (cursor == error) return FALSE; - if (frida_is_empty_function (GUM_ADDRESS (*(gpointer *)GSIZE_TO_POINTER (cursor))) && - frida_is_empty_function (GUM_ADDRESS (*(gpointer *)GSIZE_TO_POINTER (cursor + GLIB_SIZEOF_VOID_P)))) + if (frida_is_empty_function (GUM_ADDRESS (*(gpointer *) GSIZE_TO_POINTER (cursor))) && + frida_is_empty_function (GUM_ADDRESS (*(gpointer *) GSIZE_TO_POINTER (cursor + GLIB_SIZEOF_VOID_P)))) *shift = cursor - GPOINTER_TO_SIZE (vtable); else *shift = cursor - GPOINTER_TO_SIZE (vtable) - 2 * GLIB_SIZEOF_VOID_P; diff --git a/lib/payload/unwind-sitter.vala b/lib/payload/unwind-sitter.vala index 575904c4d..53b7824da 100644 --- a/lib/payload/unwind-sitter.vala +++ b/lib/payload/unwind-sitter.vala @@ -23,8 +23,7 @@ namespace Frida { dyld_find_unwind_sections = (DyldFindUnwindSectionsFunc) Gum.Module.find_export_by_name (LIBDYLD, "_dyld_find_unwind_sections"); - interceptor.replace ((void *) dyld_find_unwind_sections, - (void *) replacement_dyld_find_unwind_sections, this); + interceptor.replace ((void *) dyld_find_unwind_sections, (void *) replacement_dyld_find_unwind_sections, this); _hook_libunwind (); } @@ -51,7 +50,7 @@ namespace Frida { #endif var is_ours = address >= range.base_address && address < range_end; if (!is_ours) - return sitter.dyld_find_unwind_sections(addr, info); + return sitter.dyld_find_unwind_sections (addr, info); _fill_unwind_sections (range.base_address, range_end, info); diff --git a/tests/labrats/exception-catcher.m b/tests/labrats/exception-catcher.m index 1ac322578..e8df902c1 100644 --- a/tests/labrats/exception-catcher.m +++ b/tests/labrats/exception-catcher.m @@ -1,19 +1,18 @@ #import int -main(int argc, char * argv[]) +main (int argc, char * argv[]) { - NSURL * url = nil; + NSURL * url = nil; - @try - { - [NSBundle bundleWithURL:url]; - } - @catch (id err) - { - NSLog(@"No worries, mate!"); - } + @try + { + [NSBundle bundleWithURL:url]; + } + @catch (id err) + { + NSLog (@"No worries, mate!"); + } - return 0; + return 0; } - diff --git a/tests/test-host-session.vala b/tests/test-host-session.vala index 59a35e378..6a55e7333 100644 --- a/tests/test-host-session.vala +++ b/tests/test-host-session.vala @@ -173,12 +173,14 @@ namespace Frida.HostSessionTest { }); GLib.Test.add_func ("/HostSession/Darwin/UnwindSitter/exceptions-on-swizzled-objc-methods-should-be-caught", () => { - var h = new Harness ((h) => Darwin.UnwindSitter.exceptions_on_swizzled_objc_methods_should_be_caught.begin (h as Harness)); + var h = new Harness ((h) => + Darwin.UnwindSitter.exceptions_on_swizzled_objc_methods_should_be_caught.begin (h as Harness)); h.run (); }); GLib.Test.add_func ("/HostSession/Darwin/UnwindSitter/exceptions-on-intercepted-objc-methods-should-be-caught", () => { - var h = new Harness ((h) => Darwin.UnwindSitter.exceptions_on_intercepted_objc_methods_should_be_caught.begin (h as Harness)); + var h = new Harness ((h) => + Darwin.UnwindSitter.exceptions_on_intercepted_objc_methods_should_be_caught.begin (h as Harness)); h.run (); }); @@ -2128,9 +2130,12 @@ namespace Frida.HostSessionTest { try { var device_manager = new DeviceManager (); var device = yield device_manager.get_device_by_type (DeviceType.LOCAL); - var process = Frida.Test.Process.create (Frida.Test.Labrats.path_to_executable ("exception-catcher")); + var process = Frida.Test.Process.create ( + Frida.Test.Labrats.path_to_executable ("exception-catcher")); - /* TODO: improve injector to handle injection into a process that hasn't yet finished initializing */ + /* + * TODO: Improve injector to handle injection into a process that hasn't yet finished initializing. + */ Thread.usleep (50000); var session = yield device.attach (process.id); @@ -2186,9 +2191,12 @@ namespace Frida.HostSessionTest { try { var device_manager = new DeviceManager (); var device = yield device_manager.get_device_by_type (DeviceType.LOCAL); - var process = Frida.Test.Process.create (Frida.Test.Labrats.path_to_executable ("exception-catcher")); + var process = Frida.Test.Process.create ( + Frida.Test.Labrats.path_to_executable ("exception-catcher")); - /* TODO: improve injector to handle injection into a process that hasn't yet finished initializing */ + /* + * TODO: Improve injector to handle injection into a process that hasn't yet finished initializing. + */ Thread.usleep (50000); var session = yield device.attach (process.id);