Skip to content

Commit ddcfa3d

Browse files
authored
Fix Binder IPC side-channel detection in CallBooleanMethodV hook (#655)
This side-channel attack is obvious from the repeating logs: An isolated service (`com.reveny.nativecheck.app.isolated.IsolatedService`) of `Android-Native-Root-Detector` v7.7.0 intentionally spams Binder transactions to trigger our IPC hook. In the previous implementation, if a transaction failed, the caller's ID was stored in `g_last_failed_id`. However, the state was immediately cleared on the caller's next transaction. This created a predictable, alternating loop (Intercept -> Fail -> Bypass/Clear -> Intercept) that allowed the isolated process to detect the presence of the hook via timing/behavioral observation. We fix the flaw by keeping the failing caller in a persistent bypassed state. `g_last_failed_id` is now only reset when when the brigde approves the last connection. This effectively breaks the loop and silences the side-channel leak against continuous transaction spam. Additionally, this commit includes minor fixes discovered during debugging: - module.cpp: Fix invalid fmt placeholder (`%d` -> `{}`) in isolated process log. - ManagerService.kt: Fix logical order to save verbose logging preference before applying the LogcatMonitor state.
1 parent 9d89508 commit ddcfa3d

3 files changed

Lines changed: 12 additions & 15 deletions

File tree

daemon/src/main/kotlin/org/matrix/vector/daemon/ipc/ManagerService.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,8 +239,8 @@ object ManagerService : ILSPManagerService.Stub() {
239239
override fun isVerboseLog() = PreferenceStore.isVerboseLogEnabled() || BuildConfig.DEBUG
240240

241241
override fun setVerboseLog(enabled: Boolean) {
242-
if (isVerboseLog()) LogcatMonitor.startVerbose() else LogcatMonitor.stopVerbose()
243242
PreferenceStore.setVerboseLog(enabled)
243+
if (isVerboseLog()) LogcatMonitor.startVerbose() else LogcatMonitor.stopVerbose()
244244
}
245245

246246
override fun getVerboseLog() =

zygisk/src/main/cpp/ipc_bridge.cpp

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -476,8 +476,11 @@ jboolean IPCBridge::ExecTransact_Replace(jboolean *res, JNIEnv *env, jobject obj
476476
if (*res == JNI_FALSE) {
477477
uint64_t caller_id = BinderCaller::GetId();
478478
if (caller_id != 0) {
479+
// LOGV("Caller {} rejected by bridge service.", caller_id);
479480
g_last_failed_id.store(caller_id, std::memory_order_relaxed);
480481
}
482+
} else {
483+
g_last_failed_id.store(~0, std::memory_order_relaxed);
481484
}
482485
return true; // Return true to indicate we handled the call.
483486
}
@@ -486,23 +489,17 @@ jboolean IPCBridge::ExecTransact_Replace(jboolean *res, JNIEnv *env, jobject obj
486489

487490
jboolean JNICALL IPCBridge::CallBooleanMethodV_Hook(JNIEnv *env, jobject obj, jmethodID methodId,
488491
va_list args) {
489-
uint64_t current_caller_id = BinderCaller::GetId();
490-
if (current_caller_id != 0) {
491-
uint64_t last_failed = g_last_failed_id.load(std::memory_order_relaxed);
492-
// If this caller is the one that just failed,
493-
// skip interception and go straight to the original function.
494-
if (current_caller_id == last_failed) {
495-
// We "consume" the failed state by resetting it, so the *next* call is not skipped.
496-
g_last_failed_id.store(~0, std::memory_order_relaxed);
497-
return GetInstance().call_boolean_method_v_backup_(env, obj, methodId, args);
498-
}
499-
}
500-
501492
// Check if the method being called is the one we want to intercept: Binder.execTransact()
502493
if (methodId == GetInstance().exec_transact_backup_method_id_) {
494+
uint64_t current_caller_id = BinderCaller::GetId();
495+
503496
jboolean res = false;
504497
// Attempt to handle the transaction with our replacement logic.
505-
if (ExecTransact_Replace(&res, env, obj, args)) {
498+
if (current_caller_id != 0 &&
499+
// If this caller is the one that just failed,
500+
// skip interception and go straight to the original function.
501+
current_caller_id != g_last_failed_id.load(std::memory_order_relaxed) &&
502+
ExecTransact_Replace(&res, env, obj, args)) {
506503
return res; // If we handled it, return the result directly.
507504
}
508505
// If not handled, fall through to call the original method.

zygisk/src/main/cpp/module.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ void VectorModule::preAppSpecialize(zygisk::AppSpecializeArgs *args) {
288288
if ((app_id >= FIRST_ISOLATED_UID && app_id <= LAST_ISOLATED_UID) ||
289289
(app_id >= FIRST_APP_ZYGOTE_ISOLATED_UID && app_id <= LAST_APP_ZYGOTE_ISOLATED_UID) ||
290290
app_id == SHARED_RELRO_UID) {
291-
LOGV("Skipping injection for '{}': is an isolated process (UID: %d).", nice_name_str.get(),
291+
LOGV("Skipping injection for '{}': is an isolated process (UID: {}).", nice_name_str.get(),
292292
app_id);
293293
return;
294294
}

0 commit comments

Comments
 (0)