Skip to content

Fix potential security flaw in creation of sockets/directories #4

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 15, 2011
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions db.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand All @@ -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,
Expand All @@ -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) {
Expand All @@ -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;
}
43 changes: 16 additions & 27 deletions su.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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")) {
Expand Down Expand Up @@ -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");
Expand All @@ -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);
Expand All @@ -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();
}
Expand All @@ -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();
Expand Down
8 changes: 2 additions & 6 deletions su.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,12 @@
#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"

#define VERSION "3.0.1"
#define VERSION_CODE 12
#define VERSION "3.0.3"
#define VERSION_CODE 14

#define DATABASE_VERSION 6

Expand Down