From 448384af064cbcb659638ab38982a7eac66f2c8f Mon Sep 17 00:00:00 2001
From: canyie <a1364259@163.com>
Date: Sun, 3 Apr 2022 13:09:23 +0800
Subject: [PATCH] Guard su request IPC

Previously `read_string()` calls `std::string.resize()` with a int read from remote process. When I/O error occurs, -1 will be used for resizing the string, `std::bad_alloc` is thrown and since magisk is compiled with `-fno-exceptions`, it will crash the whole daemon process.

May fix topjohnwu#5681
---
 native/jni/core/socket.cpp    |  6 ++++--
 native/jni/include/socket.hpp |  2 +-
 native/jni/su/pts.cpp         |  2 +-
 native/jni/su/su_daemon.cpp   | 10 +++++++---
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/native/jni/core/socket.cpp b/native/jni/core/socket.cpp
index 939c3e866..9950b2a82 100644
--- a/native/jni/core/socket.cpp
+++ b/native/jni/core/socket.cpp
@@ -158,11 +158,13 @@ void write_int_be(int fd, int val) {
     xwrite(fd, &nl, sizeof(nl));
 }
 
-void read_string(int fd, std::string &str) {
+bool read_string(int fd, std::string &str) {
     int len = read_int(fd);
     str.clear();
+    if (len < 0)
+        return false;
     str.resize(len);
-    xxread(fd, str.data(), len);
+    return xxread(fd, str.data(), len) == len;
 }
 
 string read_string(int fd) {
diff --git a/native/jni/include/socket.hpp b/native/jni/include/socket.hpp
index ba35f0bb3..149391fd6 100644
--- a/native/jni/include/socket.hpp
+++ b/native/jni/include/socket.hpp
@@ -21,5 +21,5 @@ int read_int_be(int fd);
 void write_int(int fd, int val);
 void write_int_be(int fd, int val);
 std::string read_string(int fd);
-void read_string(int fd, std::string &str);
+bool read_string(int fd, std::string &str);
 void write_string(int fd, std::string_view str);
diff --git a/native/jni/su/pts.cpp b/native/jni/su/pts.cpp
index fc1ee85a2..2e93636c9 100644
--- a/native/jni/su/pts.cpp
+++ b/native/jni/su/pts.cpp
@@ -86,7 +86,7 @@ int pts_open(char *slave_name, size_t slave_name_size) {
         goto error;
 
     // Get the slave name
-    if (ptsname_r(fdm, slave_name, slave_name_size-1))
+    if (ptsname_r(fdm, slave_name, slave_name_size - 1))
         goto error;
 
     slave_name[slave_name_size - 1] = '\0';
diff --git a/native/jni/su/su_daemon.cpp b/native/jni/su/su_daemon.cpp
index 9cf090826..2b928f3aa 100644
--- a/native/jni/su/su_daemon.cpp
+++ b/native/jni/su/su_daemon.cpp
@@ -220,9 +220,13 @@ void su_daemon_handler(int client, const sock_cred *cred) {
     };
 
     // Read su_request
-    xxread(client, &ctx.req, sizeof(su_req_base));
-    read_string(client, ctx.req.shell);
-    read_string(client, ctx.req.command);
+    if (xxread(client, &ctx.req, sizeof(su_req_base)) < 0 || !read_string(client, ctx.req.shell) || !read_string(client, ctx.req.command)) {
+        LOGW("su: remote process probably died, abort\n");
+        ctx.info.reset();
+        write_int(client, DENY);
+        close(client);
+        return;
+    }
 
     // If still not determined, ask manager
     if (ctx.info->access.policy == QUERY) {