From b3242322fde5574b28ce187551eb38a3a747514f Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Sat, 9 Feb 2019 15:02:46 -0500 Subject: [PATCH] Harden socket verification - Do not allow connections to magiskd from binaries other than the one started the server - Do not allow connections to magisklogd without root access --- native/jni/core/daemon.cpp | 27 ++++++++++++++++++++------- native/jni/core/log_daemon.cpp | 7 +++++++ native/jni/core/socket.cpp | 7 ++++++- native/jni/include/daemon.h | 1 + 4 files changed, 34 insertions(+), 8 deletions(-) diff --git a/native/jni/core/daemon.cpp b/native/jni/core/daemon.cpp index 4904da986..34d66e7c8 100644 --- a/native/jni/core/daemon.cpp +++ b/native/jni/core/daemon.cpp @@ -23,21 +23,30 @@ #include "flags.h" int SDK_INT = -1; +struct stat SERVER_STAT; -static void get_client_cred(int fd, struct ucred *cred) { - socklen_t ucred_length = sizeof(*cred); - if(getsockopt(fd, SOL_SOCKET, SO_PEERCRED, cred, &ucred_length)) - PLOGE("getsockopt"); +static void verify_client(int client, pid_t pid) { + // Verify caller is the same as server + char path[32]; + sprintf(path, "/proc/%d/exe", pid); + struct stat st{}; + stat(path, &st); + if (st.st_dev != SERVER_STAT.st_dev || st.st_ino != SERVER_STAT.st_ino) { + close(client); + pthread_exit(nullptr); + } } static void *request_handler(void *args) { int client = *((int *) args); delete (int *) args; - int req = read_int(client); struct ucred credential; get_client_cred(client, &credential); + if (credential.uid != 0) + verify_client(client, credential.pid); + int req = read_int(client); switch (req) { case MAGISKHIDE: case POST_FS_DATA: @@ -103,6 +112,11 @@ static void main_daemon() { xdup2(fd, STDIN_FILENO); close(fd); + LOGI("Magisk v" xstr(MAGISK_VERSION) "(" xstr(MAGISK_VER_CODE) ") daemon started\n"); + + // Get server stat + stat("/proc/self/exe", &SERVER_STAT); + // Get API level parse_prop_file("/system/build.prop", [](auto key, auto val) -> bool { if (strcmp(key, "ro.build.version.sdk") == 0) { @@ -119,7 +133,6 @@ static void main_daemon() { if (xbind(fd, (struct sockaddr*) &sun, len)) exit(1); xlisten(fd, 10); - LOGI("Magisk v" xstr(MAGISK_VERSION) "(" xstr(MAGISK_VER_CODE) ") daemon started\n"); // Change process name strcpy(argv0, "magiskd"); @@ -138,7 +151,7 @@ static void main_daemon() { sigaction(SIGPIPE, &act, nullptr); // Loop forever to listen for requests - while(1) { + while(true) { int *client = new int; *client = xaccept4(fd, nullptr, nullptr, SOCK_CLOEXEC); pthread_t thread; diff --git a/native/jni/core/log_daemon.cpp b/native/jni/core/log_daemon.cpp index 3f8b1368c..c179cdf91 100644 --- a/native/jni/core/log_daemon.cpp +++ b/native/jni/core/log_daemon.cpp @@ -166,6 +166,13 @@ static void log_daemon() { xlisten(sockfd, 10); while(true) { int fd = xaccept4(sockfd, nullptr, nullptr, SOCK_CLOEXEC); + struct ucred credential; + get_client_cred(fd, &credential); + if (credential.uid != 0) { + // Do not allow non root clients + close(fd); + continue; + } switch(read_int(fd)) { case HIDE_CONNECT: pthread_mutex_lock(&lock); diff --git a/native/jni/core/socket.cpp b/native/jni/core/socket.cpp index 3ad18eff9..d201dc4ce 100644 --- a/native/jni/core/socket.cpp +++ b/native/jni/core/socket.cpp @@ -11,7 +11,7 @@ #include "utils.h" #include "magisk.h" -#define ABS_SOCKET_LEN(sun) (sizeof(sun->sun_family) + strlen(sun->sun_path + 1) + 1) +#define ABS_SOCKET_LEN(sun) (sizeof(sa_family_t) + strlen(sun->sun_path + 1) + 1) socklen_t setup_sockaddr(struct sockaddr_un *sun, const char *name) { memset(sun, 0, sizeof(*sun)); @@ -38,6 +38,11 @@ int socket_accept(int sockfd, int timeout) { return xpoll(&pfd, 1, timeout * 1000) <= 0 ? -1 : xaccept4(sockfd, NULL, NULL, SOCK_CLOEXEC); } +void get_client_cred(int fd, struct ucred *cred) { + socklen_t ucred_length = sizeof(*cred); + getsockopt(fd, SOL_SOCKET, SO_PEERCRED, cred, &ucred_length); +} + /* * Receive a file descriptor from a Unix socket. * Contributed by @mkasick diff --git a/native/jni/include/daemon.h b/native/jni/include/daemon.h index b2c468d71..21c0f44a7 100644 --- a/native/jni/include/daemon.h +++ b/native/jni/include/daemon.h @@ -48,6 +48,7 @@ bool start_log_daemon(); socklen_t setup_sockaddr(struct sockaddr_un *sun, const char *name); int create_rand_socket(struct sockaddr_un *sun); int socket_accept(int sockfd, int timeout); +void get_client_cred(int fd, struct ucred *cred); int recv_fd(int sockfd); void send_fd(int sockfd, int fd); int read_int(int fd);