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
This commit is contained in:
canyie 2022-04-03 13:09:23 +08:00 committed by John Wu
parent 3f840f53a0
commit 448384af06
4 changed files with 13 additions and 7 deletions

View File

@ -158,11 +158,13 @@ void write_int_be(int fd, int val) {
xwrite(fd, &nl, sizeof(nl)); 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); int len = read_int(fd);
str.clear(); str.clear();
if (len < 0)
return false;
str.resize(len); str.resize(len);
xxread(fd, str.data(), len); return xxread(fd, str.data(), len) == len;
} }
string read_string(int fd) { string read_string(int fd) {

View File

@ -21,5 +21,5 @@ int read_int_be(int fd);
void write_int(int fd, int val); void write_int(int fd, int val);
void write_int_be(int fd, int val); void write_int_be(int fd, int val);
std::string read_string(int fd); 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); void write_string(int fd, std::string_view str);

View File

@ -86,7 +86,7 @@ int pts_open(char *slave_name, size_t slave_name_size) {
goto error; goto error;
// Get the slave name // 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; goto error;
slave_name[slave_name_size - 1] = '\0'; slave_name[slave_name_size - 1] = '\0';

View File

@ -220,9 +220,13 @@ void su_daemon_handler(int client, const sock_cred *cred) {
}; };
// Read su_request // Read su_request
xxread(client, &ctx.req, sizeof(su_req_base)); if (xxread(client, &ctx.req, sizeof(su_req_base)) < 0 || !read_string(client, ctx.req.shell) || !read_string(client, ctx.req.command)) {
read_string(client, ctx.req.shell); LOGW("su: remote process probably died, abort\n");
read_string(client, ctx.req.command); ctx.info.reset();
write_int(client, DENY);
close(client);
return;
}
// If still not determined, ask manager // If still not determined, ask manager
if (ctx.info->access.policy == QUERY) { if (ctx.info->access.policy == QUERY) {