Merge pull request ChainsDD#4 from git-core/master

ChainsDD · ChainsDD · commit 690dcc6517ec · 2011-12-15T12:39:39.000-08:00
Fix potential security flaw in creation of sockets/directories
diff --git a/db.c b/db.c
@@ -28,8 +28,7 @@
 sqlite3 *database_init()
 {
     sqlite3 *db;
-    int version, rc, databaseStatus = 0;
-    char *zErrMsg = 0;
+    int rc;
 
     rc = sqlite3_open_v2(REQUESTOR_DATABASE_PATH, &db, SQLITE_OPEN_READONLY, NULL);
     if ( rc ) {
@@ -48,8 +47,7 @@ int database_check(sqlite3 *db, struct su_initiator *from, struct su_request *to
     char *zErrmsg;
     char **result;
     int nrow,ncol;
-    int allow;
-    struct timeval tv;
+    int allow = DB_INTERACTIVE;
 
     sqlite3_snprintf(
         sizeof(sql), sql,
@@ -70,7 +68,7 @@ int database_check(sqlite3 *db, struct su_initiator *from, struct su_request *to
     }
     
     if (nrow == 0 || ncol != 3)
-        return DB_INTERACTIVE;
+        goto out;
         
     if (strcmp(result[0], "_id") == 0 && strcmp(result[2], "allow") == 0) {
         if (strcmp(result[5], "1") == 0) {
@@ -80,10 +78,10 @@ int database_check(sqlite3 *db, struct su_initiator *from, struct su_request *to
         } else {
             allow = DB_DENY;
         }
-        return allow;
     }
 
+out:
     sqlite3_free_table(result);
     
-    return DB_INTERACTIVE;
+    return allow;
 }
diff --git a/su.c b/su.c
@@ -71,6 +71,7 @@ static int from_init(struct su_initiator *from)
     int fd;
     ssize_t len;
     int i;
+    int err;
 
     from->uid = getuid();
     from->pid = getppid();
@@ -83,9 +84,10 @@ static int from_init(struct su_initiator *from)
         return -1;
     }
     len = read(fd, args, sizeof(args));
+    err = errno;
     close(fd);
     if (len < 0 || len == sizeof(args)) {
-        PLOGE("Reading command line");
+        PLOGEV("Reading command line", err);
         return -1;
     }
 
@@ -144,10 +146,10 @@ static void cleanup_signal(int sig)
     exit(sig);
 }
 
-static int socket_create_temp(unsigned req_uid)
+static int socket_create_temp(void)
 {
     static char buf[PATH_MAX];
-    int fd, err;
+    int fd;
 
     struct sockaddr_un sun;
 
@@ -174,18 +176,6 @@ static int socket_create_temp(unsigned req_uid)
         }
     }
 
-    if (chmod(sun.sun_path, 0600) < 0) {
-        PLOGE("chmod(socket)");
-        unlink(sun.sun_path);
-        return -1;
-    }
-
-    if (chown(sun.sun_path, req_uid, req_uid) < 0) {
-        PLOGE("chown(socket)");
-        unlink(sun.sun_path);
-        return -1;
-    }
-
     if (listen(fd, 1) < 0) {
         PLOGE("listen");
         return -1;
@@ -222,7 +212,6 @@ static int socket_accept(int serv_fd)
 static int socket_receive_result(int serv_fd, char *result, ssize_t result_len)
 {
     ssize_t len;
-    char buf[64];
     
     for (;;) {
         int fd = socket_accept(serv_fd);
@@ -275,19 +264,19 @@ static void deny(void)
     exit(EXIT_FAILURE);
 }
 
-static void allow(char *shell)
+static void allow(char *shell, mode_t mask)
 {
     struct su_initiator *from = &su_from;
     struct su_request *to = &su_to;
     char *exe = NULL;
 
+    umask(mask);
     send_intent(&su_from, &su_to, "", 1, 1);
 
     if (!strcmp(shell, "")) {
         strcpy(shell , "/system/bin/sh");
     }
     exe = strrchr (shell, '/') + 1;
-    setgroups(0, NULL);
     setresgid(to->uid, to->uid, to->uid);
     setresuid(to->uid, to->uid, to->uid);
     LOGD("%u %s executing %u %s using shell %s : %s", from->uid, from->bin,
@@ -307,7 +296,7 @@ int main(int argc, char *argv[])
     static int socket_serv_fd = -1;
     char buf[64], shell[PATH_MAX], *result;
     int i, dballow;
-    unsigned req_uid;
+    mode_t orig_umask;
 
     for (i = 1; i < argc; i++) {
         if (!strcmp(argv[i], "-c") || !strcmp(argv[i], "--command")) {
@@ -356,8 +345,10 @@ int main(int argc, char *argv[])
         deny();
     }
 
+    orig_umask = umask(027);
+
     if (su_from.uid == AID_ROOT || su_from.uid == AID_SHELL)
-        allow(shell);
+        allow(shell, orig_umask);
 
     if (stat(REQUESTOR_DATA_PATH, &st) < 0) {
         PLOGE("stat");
@@ -371,10 +362,8 @@ int main(int argc, char *argv[])
         deny();
     }
 
-    req_uid = st.st_uid;
-
-    if (mkdir(REQUESTOR_CACHE_PATH, 0771) >= 0) {
-        chown(REQUESTOR_CACHE_PATH, req_uid, req_uid);
+    if (mkdir(REQUESTOR_CACHE_PATH, 0770) >= 0) {
+        chown(REQUESTOR_CACHE_PATH, st.st_uid, st.st_gid);
     }
 
     setgroups(0, NULL);
@@ -400,12 +389,12 @@ int main(int argc, char *argv[])
 
     switch (dballow) {
         case DB_DENY: deny();
-        case DB_ALLOW: allow(shell);
+        case DB_ALLOW: allow(shell, orig_umask);
         case DB_INTERACTIVE: break;
         default: deny();
     }
     
-    socket_serv_fd = socket_create_temp(req_uid);
+    socket_serv_fd = socket_create_temp();
     if (socket_serv_fd < 0) {
         deny();
     }
@@ -432,7 +421,7 @@ int main(int argc, char *argv[])
     if (!strcmp(result, "DENY")) {
         deny();
     } else if (!strcmp(result, "ALLOW")) {
-        allow(shell);
+        allow(shell, orig_umask);
     } else {
         LOGE("unknown response from Superuser Requestor: %s", result);
         deny();
diff --git a/su.h b/su.h
@@ -24,10 +24,6 @@
 #define REQUESTOR_DATABASES_PATH REQUESTOR_DATA_PATH "/databases"
 #define REQUESTOR_DATABASE_PATH REQUESTOR_DATABASES_PATH "/permissions.sqlite"
 
-#define APPS_TABLE_DEFINITION "CREATE TABLE IF NOT EXISTS apps (_id INTEGER, uid INTEGER, package TEXT, name TEXT, exec_uid INTEGER, exec_cmd TEXT, allow INTEGER, PRIMARY KEY (_id), UNIQUE (uid,exec_uid,exec_cmd));"
-#define LOGS_TABLE_DEFINITION "CREATE TABLE IF NOT EXISTS logs (_id INTEGER, uid INTEGER, name INTEGER, app_id INTEGER, date INTEGER, type INTEGER, PRIMARY KEY (_id));"
-#define PREFS_TABLE_DEFINITION "CREATE TABLE IF NOT EXISTS prefs (_id INTEGER, key TEXT, value TEXT, PRIMARY KEY(_id));"
-
 #define DEFAULT_COMMAND "/system/bin/sh"
 
 #define SOCKET_PATH_TEMPLATE REQUESTOR_CACHE_PATH "/.socketXXXXXX"
