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.
This commit is contained in:
ThePedroo
2025-04-27 19:50:59 -03:00
parent 0c0f65998d
commit 0930c8cca4
11 changed files with 47 additions and 34 deletions

View File

@@ -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);

View File

@@ -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));

View File

@@ -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);

View File

@@ -4,6 +4,8 @@
#include <stdint.h>
int read_fd(int fd);
ssize_t write_string(int fd, const char *str);
char *read_string(int fd);

View File

@@ -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,

View File

@@ -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)

View File

@@ -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;

View File

@@ -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);

View File

@@ -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;

View File

@@ -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);

View File

@@ -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;
}
}