From bc9a6dd3916713073ddd54c4743e2a57f9499b27 Mon Sep 17 00:00:00 2001 From: Your Name Date: Tue, 2 Jan 2024 17:16:34 +0000 Subject: [PATCH] Review Changes --- bindings/gumjs/gumquickinterceptor.c | 139 +++++++++++---------------- bindings/gumjs/gumv8interceptor.cpp | 104 +++++++------------- 2 files changed, 88 insertions(+), 155 deletions(-) diff --git a/bindings/gumjs/gumquickinterceptor.c b/bindings/gumjs/gumquickinterceptor.c index c86a935552..fd9070259e 100644 --- a/bindings/gumjs/gumquickinterceptor.c +++ b/bindings/gumjs/gumquickinterceptor.c @@ -164,6 +164,10 @@ static void gum_quick_interceptor_detach (GumQuickInterceptor * self, GumQuickInvocationListener * listener); GUMJS_DECLARE_FUNCTION (gumjs_interceptor_detach_all) GUMJS_DECLARE_FUNCTION (gumjs_interceptor_replace) +static void gum_quick_replace_entry_new (GumQuickInterceptor * self, + gpointer target, JSValue replacement_value); +static JSValue gum_quick_handle_replace_ret (GumQuickCore * core, + gpointer target, GumReplaceReturn replace_ret); GUMJS_DECLARE_FUNCTION (gumjs_interceptor_replace_fast) static void gum_quick_replace_entry_free (GumQuickReplaceEntry * entry); static void gum_quick_replace_entry_revert_and_free ( @@ -645,20 +649,39 @@ GUMJS_DEFINE_FUNCTION (gumjs_interceptor_replace) GumQuickInterceptor * self; gpointer target, replacement_function, replacement_data; JSValue replacement_value; - GumQuickReplaceEntry * entry = NULL; GumReplaceReturn replace_ret; - GumQuickNativeCallback * c; self = gumjs_get_parent_module (core); replacement_data = NULL; if (!_gum_quick_args_parse (args, "pO|p", &target, &replacement_value, &replacement_data)) - goto propagate_exception; + return JS_EXCEPTION; if (!_gum_quick_native_pointer_get (ctx, replacement_value, core, &replacement_function)) - goto propagate_exception; + return JS_EXCEPTION; + + replace_ret = gum_interceptor_replace (self->interceptor, target, + replacement_function, replacement_data, NULL); + if (replace_ret != GUM_REPLACE_OK) + return gum_quick_handle_replace_ret (core, target, replace_ret); + + gum_quick_replace_entry_new (self, target, replacement_value); + + return JS_UNDEFINED; +} + + +static void +gum_quick_replace_entry_new (GumQuickInterceptor * self, + gpointer target, + JSValue replacement_value) +{ + GumQuickCore * core = self->core; + GumQuickReplaceEntry * entry = NULL; + GumQuickNativeCallback * c; + JSContext * ctx = core->ctx; entry = g_slice_new (GumQuickReplaceEntry); entry->interceptor = self->interceptor; @@ -666,48 +689,38 @@ GUMJS_DEFINE_FUNCTION (gumjs_interceptor_replace) entry->replacement = JS_DupValue (ctx, replacement_value); entry->ctx = ctx; - replace_ret = gum_interceptor_replace (self->interceptor, target, - replacement_function, replacement_data, NULL); - if (replace_ret != GUM_REPLACE_OK) - goto unable_to_replace; - c = JS_GetOpaque (entry->replacement, core->native_callback_class); if (c != NULL) c->interceptor_replacement_count++; g_hash_table_insert (self->replacement_by_address, target, entry); +} - return JS_UNDEFINED; - -unable_to_replace: - { - switch (replace_ret) - { - case GUM_REPLACE_WRONG_SIGNATURE: - _gum_quick_throw (ctx, "unable to intercept function at %p; " - "please file a bug", target); - break; - case GUM_REPLACE_ALREADY_REPLACED: - _gum_quick_throw_literal (ctx, "already replaced this function"); - break; - case GUM_REPLACE_POLICY_VIOLATION: - _gum_quick_throw_literal (ctx, "not permitted by code-signing policy"); - break; - case GUM_REPLACE_WRONG_TYPE: - _gum_quick_throw_literal (ctx, "wrong type"); - break; - default: - g_assert_not_reached (); - } - - goto propagate_exception; - } -propagate_exception: +static JSValue +gum_quick_handle_replace_ret (GumQuickCore * core, + gpointer target, + GumReplaceReturn replace_ret) +{ + JSContext * ctx = core->ctx; + switch (replace_ret) { - gum_quick_replace_entry_free (entry); - - return JS_EXCEPTION; + case GUM_REPLACE_WRONG_SIGNATURE: + _gum_quick_throw (ctx, "unable to intercept function at %p; " + "please file a bug", target); + break; + case GUM_REPLACE_ALREADY_REPLACED: + _gum_quick_throw_literal (ctx, "already replaced this function"); + break; + case GUM_REPLACE_POLICY_VIOLATION: + _gum_quick_throw_literal (ctx, "not permitted by code-signing policy"); + break; + case GUM_REPLACE_WRONG_TYPE: + _gum_quick_throw_literal (ctx, "wrong type"); + break; + default: + g_assert_not_reached (); } + return JS_EXCEPTION; } static void @@ -726,68 +739,26 @@ GUMJS_DEFINE_FUNCTION (gumjs_interceptor_replace_fast) GumQuickInterceptor * self; gpointer target, replacement_function, original_function; JSValue replacement_value; - GumQuickReplaceEntry * entry = NULL; GumReplaceReturn replace_ret; - GumQuickNativeCallback * c; self = gumjs_get_parent_module (core); if (!_gum_quick_args_parse (args, "pO", &target, &replacement_value)) - goto propagate_exception; + return JS_EXCEPTION; if (!_gum_quick_native_pointer_get (ctx, replacement_value, core, &replacement_function)) - goto propagate_exception; - - entry = g_slice_new (GumQuickReplaceEntry); - entry->interceptor = self->interceptor; - entry->target = target; - entry->replacement = JS_DupValue (ctx, replacement_value); - entry->ctx = ctx; + return JS_EXCEPTION; replace_ret = gum_interceptor_replace_fast (self->interceptor, target, replacement_function, &original_function); if (replace_ret != GUM_REPLACE_OK) - goto unable_to_replace; + return gum_quick_handle_replace_ret (core, target, replace_ret); - c = JS_GetOpaque (entry->replacement, core->native_callback_class); - if (c != NULL) - c->interceptor_replacement_count++; - - g_hash_table_insert (self->replacement_by_address, target, entry); + gum_quick_replace_entry_new (self, target, replacement_value); return _gum_quick_native_pointer_new (ctx, GSIZE_TO_POINTER (original_function), core); - -unable_to_replace: - { - switch (replace_ret) - { - case GUM_REPLACE_WRONG_SIGNATURE: - _gum_quick_throw (ctx, "unable to intercept function at %p; " - "please file a bug", target); - break; - case GUM_REPLACE_ALREADY_REPLACED: - _gum_quick_throw_literal (ctx, "already replaced this function"); - break; - case GUM_REPLACE_POLICY_VIOLATION: - _gum_quick_throw_literal (ctx, "not permitted by code-signing policy"); - break; - case GUM_REPLACE_WRONG_TYPE: - _gum_quick_throw_literal (ctx, "wrong type"); - break; - default: - g_assert_not_reached (); - } - - goto propagate_exception; - } -propagate_exception: - { - gum_quick_replace_entry_free (entry); - - return JS_EXCEPTION; - } } static void diff --git a/bindings/gumjs/gumv8interceptor.cpp b/bindings/gumjs/gumv8interceptor.cpp index c0bb0db8ab..b65a568cf2 100644 --- a/bindings/gumjs/gumv8interceptor.cpp +++ b/bindings/gumjs/gumv8interceptor.cpp @@ -145,6 +145,9 @@ static void gum_v8_invocation_listener_destroy ( GumV8InvocationListener * listener); GUMJS_DECLARE_FUNCTION (gumjs_interceptor_detach_all) GUMJS_DECLARE_FUNCTION (gumjs_interceptor_replace) +static void gum_v8_handle_replace_ret (GumV8Interceptor * self, + gpointer target, Local replacement_value, + GumReplaceReturn replace_ret); GUMJS_DECLARE_FUNCTION (gumjs_interceptor_replace_fast) static void gum_v8_replace_entry_free (GumV8ReplaceEntry * entry); GUMJS_DECLARE_FUNCTION (gumjs_interceptor_revert) @@ -698,40 +701,44 @@ GUMJS_DEFINE_FUNCTION (gumjs_interceptor_replace) if (!_gum_v8_args_parse (args, "pp|p", &target, &replacement_function, &replacement_data)) return; - auto replacement_function_value = info[1]; - - auto entry = g_slice_new (GumV8ReplaceEntry); - entry->interceptor = module->interceptor; - entry->target = target; - entry->replacement = new Global (isolate, replacement_function_value); auto replace_ret = gum_interceptor_replace (module->interceptor, target, replacement_function, replacement_data, NULL); - if (replace_ret == GUM_REPLACE_OK) - { - auto native_callback = Local::New (isolate, - *core->native_callback); - auto instance = replacement_function_value.As (); - if (native_callback->HasInstance (instance)) - { - auto callback = (GumV8NativeCallback *) - instance->GetInternalField (1).As ()->Value (); - callback->interceptor_replacement_count++; - } + gum_v8_handle_replace_ret (module, target, info[1], replace_ret); +} - g_hash_table_insert (module->replacement_by_address, target, entry); - } - else - { - delete entry->replacement; - g_slice_free (GumV8ReplaceEntry, entry); - } +static void +gum_v8_handle_replace_ret (GumV8Interceptor * self, + gpointer target, + Local replacement_value, + GumReplaceReturn replace_ret) +{ + GumV8Core * core = self->core; + auto isolate = core->isolate; switch (replace_ret) { case GUM_REPLACE_OK: + { + auto entry = g_slice_new (GumV8ReplaceEntry); + entry->interceptor = self->interceptor; + entry->target = target; + entry->replacement = new Global (isolate, replacement_value); + + g_hash_table_insert (self->replacement_by_address, target, entry); + + auto native_callback = Local::New (isolate, + *core->native_callback); + auto instance = replacement_value.As (); + if (native_callback->HasInstance (instance)) + { + auto callback = (GumV8NativeCallback *) + instance->GetInternalField (1).As ()->Value (); + callback->interceptor_replacement_count++; + } break; + } case GUM_REPLACE_WRONG_SIGNATURE: { _gum_v8_throw_ascii (isolate, "unable to intercept function at %p; " @@ -758,62 +765,17 @@ GUMJS_DEFINE_FUNCTION (gumjs_interceptor_replace_fast) gpointer target, replacement_function, original_function; if (!_gum_v8_args_parse (args, "pp", &target, &replacement_function)) return; - auto replacement_function_value = info[1]; - - auto entry = g_slice_new (GumV8ReplaceEntry); - entry->interceptor = module->interceptor; - entry->target = target; - entry->replacement = new Global (isolate, replacement_function_value); auto replace_ret = gum_interceptor_replace_fast (module->interceptor, target, replacement_function, &original_function); + gum_v8_handle_replace_ret (module, target, info[1], replace_ret); + if (replace_ret == GUM_REPLACE_OK) { - auto native_callback = Local::New (isolate, - *core->native_callback); - auto instance = replacement_function_value.As (); - if (native_callback->HasInstance (instance)) - { - auto callback = (GumV8NativeCallback *) - instance->GetInternalField (1).As ()->Value (); - callback->interceptor_replacement_count++; - } - - g_hash_table_insert (module->replacement_by_address, target, entry); - info.GetReturnValue ().Set ( _gum_v8_native_pointer_new ( GSIZE_TO_POINTER (original_function), core)); } - else - { - delete entry->replacement; - g_slice_free (GumV8ReplaceEntry, entry); - } - - switch (replace_ret) - { - case GUM_REPLACE_OK: - break; - case GUM_REPLACE_WRONG_SIGNATURE: - { - _gum_v8_throw_ascii (isolate, "unable to intercept function at %p; " - "please file a bug", target); - break; - } - case GUM_REPLACE_ALREADY_REPLACED: - _gum_v8_throw_ascii_literal (isolate, "already replaced this function"); - break; - case GUM_REPLACE_POLICY_VIOLATION: - _gum_v8_throw_ascii_literal (isolate, - "not permitted by code-signing policy"); - break; - case GUM_REPLACE_WRONG_TYPE: - _gum_v8_throw_ascii_literal (isolate, "wrong type"); - break; - default: - g_assert_not_reached (); - } } static void