From 0930c8cca4e48859706488e157a8796a7d7467d3 Mon Sep 17 00:00:00 2001 From: ThePedroo Date: Sun, 27 Apr 2025 19:50:59 -0300 Subject: [PATCH] fix: ReZygiskd Magisk DenyList not checking against `process` This commit improves the precision of ReZygiskd check for Magisk if a process is in DenyList/SuList, as previously it used "package_name" instead of the correct "process" field. --- loader/src/common/daemon.c | 3 ++- loader/src/common/socket_utils.c | 19 ++++++++++++++++++ loader/src/include/daemon.h | 2 +- loader/src/include/socket_utils.h | 2 ++ loader/src/injector/hook.cpp | 2 +- zygiskd/src/constants.h | 2 ++ zygiskd/src/root_impl/common.c | 4 ++-- zygiskd/src/root_impl/common.h | 2 +- zygiskd/src/root_impl/magisk.c | 32 ++++++------------------------- zygiskd/src/root_impl/magisk.h | 2 +- zygiskd/src/zygiskd.c | 11 ++++++++++- 11 files changed, 47 insertions(+), 34 deletions(-) diff --git a/loader/src/common/daemon.c b/loader/src/common/daemon.c index dcd307f..31f8fc2 100644 --- a/loader/src/common/daemon.c +++ b/loader/src/common/daemon.c @@ -64,7 +64,7 @@ bool rezygiskd_ping() { return true; } -uint32_t rezygiskd_get_process_flags(uid_t uid) { +uint32_t rezygiskd_get_process_flags(uid_t uid, const char *const process) { int fd = rezygiskd_connect(1); if (fd == -1) { PLOGE("connection to ReZygiskd"); @@ -74,6 +74,7 @@ uint32_t rezygiskd_get_process_flags(uid_t uid) { write_uint8_t(fd, (uint8_t)GetProcessFlags); write_uint32_t(fd, (uint32_t)uid); + write_string(fd, process); uint32_t res = 0; read_uint32_t(fd, &res); diff --git a/loader/src/common/socket_utils.c b/loader/src/common/socket_utils.c index 24bddc0..29a337c 100644 --- a/loader/src/common/socket_utils.c +++ b/loader/src/common/socket_utils.c @@ -45,6 +45,25 @@ int read_fd(int fd) { return sendfd; } +ssize_t write_string(int fd, const char *str) { + size_t str_len = strlen(str); + ssize_t write_bytes = write(fd, &str_len, sizeof(size_t)); + if (write_bytes != (ssize_t)sizeof(size_t)) { + LOGE("Failed to write string length: Not all bytes were written (%zd != %zu).\n", write_bytes, sizeof(size_t)); + + return -1; + } + + write_bytes = write(fd, str, str_len); + if (write_bytes != (ssize_t)str_len) { + LOGE("Failed to write string: Promised bytes doesn't exist (%zd != %zu).\n", write_bytes, str_len); + + return -1; + } + + return write_bytes; +} + char *read_string(int fd) { size_t str_len = 0; ssize_t read_bytes = read(fd, &str_len, sizeof(size_t)); diff --git a/loader/src/include/daemon.h b/loader/src/include/daemon.h index 4bbe89c..83004b3 100644 --- a/loader/src/include/daemon.h +++ b/loader/src/include/daemon.h @@ -63,7 +63,7 @@ int rezygiskd_connect(uint8_t retry); bool rezygiskd_ping(); -uint32_t rezygiskd_get_process_flags(uid_t uid); +uint32_t rezygiskd_get_process_flags(uid_t uid, const char *const process); void rezygiskd_get_info(struct rezygisk_info *info); diff --git a/loader/src/include/socket_utils.h b/loader/src/include/socket_utils.h index 4c40559..8891335 100644 --- a/loader/src/include/socket_utils.h +++ b/loader/src/include/socket_utils.h @@ -4,6 +4,8 @@ #include int read_fd(int fd); + +ssize_t write_string(int fd, const char *str); char *read_string(int fd); diff --git a/loader/src/injector/hook.cpp b/loader/src/injector/hook.cpp index add369f..024c4d2 100644 --- a/loader/src/injector/hook.cpp +++ b/loader/src/injector/hook.cpp @@ -677,7 +677,7 @@ void ZygiskContext::run_modules_post() { void ZygiskContext::app_specialize_pre() { flags[APP_SPECIALIZE] = true; - info_flags = rezygiskd_get_process_flags(g_ctx->args.app->uid); + info_flags = rezygiskd_get_process_flags(g_ctx->args.app->uid, (const char *const)process); if (info_flags & PROCESS_IS_FIRST_STARTED) { /* INFO: To ensure we are really using a clean mount namespace, we use the first process it as reference for clean mount namespace, diff --git a/zygiskd/src/constants.h b/zygiskd/src/constants.h index a9d9f44..84fcf5e 100644 --- a/zygiskd/src/constants.h +++ b/zygiskd/src/constants.h @@ -13,6 +13,8 @@ #define lp_select(a, b) a #endif +#define PROCESS_NAME_MAX_LEN 256 + 1 + #define ZYGOTE_INJECTED lp_select(5, 4) #define DAEMON_SET_INFO lp_select(7, 6) #define DAEMON_SET_ERROR_INFO lp_select(9, 8) diff --git a/zygiskd/src/root_impl/common.c b/zygiskd/src/root_impl/common.c index 2198bdc..32c3e0c 100644 --- a/zygiskd/src/root_impl/common.c +++ b/zygiskd/src/root_impl/common.c @@ -94,7 +94,7 @@ bool uid_granted_root(uid_t uid) { } } -bool uid_should_umount(uid_t uid) { +bool uid_should_umount(uid_t uid, const char *const process) { switch (impl.impl) { case KernelSU: { return ksu_uid_should_umount(uid); @@ -103,7 +103,7 @@ bool uid_should_umount(uid_t uid) { return apatch_uid_should_umount(uid); } case Magisk: { - return magisk_uid_should_umount(uid); + return magisk_uid_should_umount(process); } default: { return false; diff --git a/zygiskd/src/root_impl/common.h b/zygiskd/src/root_impl/common.h index dedb796..7924d7b 100644 --- a/zygiskd/src/root_impl/common.h +++ b/zygiskd/src/root_impl/common.h @@ -31,7 +31,7 @@ void get_impl(struct root_impl *uimpl); bool uid_granted_root(uid_t uid); -bool uid_should_umount(uid_t uid); +bool uid_should_umount(uid_t uid, const char *const process); bool uid_is_manager(uid_t uid); diff --git a/zygiskd/src/root_impl/magisk.c b/zygiskd/src/root_impl/magisk.c index ee5d48b..4b201f0 100644 --- a/zygiskd/src/root_impl/magisk.c +++ b/zygiskd/src/root_impl/magisk.c @@ -139,37 +139,17 @@ bool magisk_uid_granted_root(uid_t uid) { return result[0] != '\0'; } -bool magisk_uid_should_umount(uid_t uid) { - char uid_str[16]; - snprintf(uid_str, sizeof(uid_str), "%d", uid); - - char *const argv_pm[] = { "pm", "list", "packages", "--uid", uid_str, NULL }; - - char result[256]; - if (!exec_command(result, sizeof(result), "/system/bin/pm", argv_pm)) { - LOGE("Failed to execute pm binary: %s\n", strerror(errno)); - errno = 0; - - /* INFO: It's better if we do NOT umount than the opposite */ - return false; - } - - if (result[0] == '\0') { - LOGE("Failed to get package name from UID %d\n", uid); - - return false; - } - - char *package_name = strtok(result + strlen("package:"), " "); - - char sqlite_cmd[256]; +bool magisk_uid_should_umount(const char *const process) { + /* INFO: PROCESS_NAME_MAX_LEN already has a +1 for NULL */ + char sqlite_cmd[51 + PROCESS_NAME_MAX_LEN]; if (is_using_sulist) - snprintf(sqlite_cmd, sizeof(sqlite_cmd), "select 1 from sulist where package_name=\"%s\" limit 1", package_name); + snprintf(sqlite_cmd, sizeof(sqlite_cmd), "SELECT 1 FROM sulist WHERE process=\"%s\" LIMIT 1", process); else - snprintf(sqlite_cmd, sizeof(sqlite_cmd), "select 1 from denylist where package_name=\"%s\" limit 1", package_name); + snprintf(sqlite_cmd, sizeof(sqlite_cmd), "SELECT 1 FROM denylist WHERE process=\"%s\" LIMIT 1", process); char *const argv[] = { "magisk", "--sqlite", sqlite_cmd, NULL }; + char result[sizeof("1=1")]; if (!exec_command(result, sizeof(result), (const char *)path_to_magisk, argv)) { LOGE("Failed to execute magisk binary: %s\n", strerror(errno)); errno = 0; diff --git a/zygiskd/src/root_impl/magisk.h b/zygiskd/src/root_impl/magisk.h index 0d47646..4b8b164 100644 --- a/zygiskd/src/root_impl/magisk.h +++ b/zygiskd/src/root_impl/magisk.h @@ -12,7 +12,7 @@ void magisk_get_existence(struct root_impl_state *state); bool magisk_uid_granted_root(uid_t uid); -bool magisk_uid_should_umount(uid_t uid); +bool magisk_uid_should_umount(const char *const process); bool magisk_uid_is_manager(uid_t uid); diff --git a/zygiskd/src/zygiskd.c b/zygiskd/src/zygiskd.c index 4132d87..3e18f6c 100644 --- a/zygiskd/src/zygiskd.c +++ b/zygiskd/src/zygiskd.c @@ -415,6 +415,15 @@ void zygiskd_start(char *restrict argv[]) { ssize_t ret = read_uint32_t(client_fd, &uid); ASSURE_SIZE_READ_BREAK("GetProcessFlags", "uid", ret, sizeof(uid)); + /* INFO: Only used for Magisk, as it saves process names and not UIDs. */ + char process[PROCESS_NAME_MAX_LEN]; + ret = read_string(client_fd, process, sizeof(process)); + if (ret == -1) { + LOGE("Failed reading process name.\n"); + + break; + } + uint32_t flags = 0; if (first_process) { flags |= PROCESS_IS_FIRST_STARTED; @@ -427,7 +436,7 @@ void zygiskd_start(char *restrict argv[]) { if (uid_granted_root(uid)) { flags |= PROCESS_GRANTED_ROOT; } - if (uid_should_umount(uid)) { + if (uid_should_umount(uid, (const char *const)process)) { flags |= PROCESS_ON_DENYLIST; } }