From bb41946dd82c582855328c4cb38c383efaf7b469 Mon Sep 17 00:00:00 2001 From: Charlie Fenton Date: Tue, 2 Oct 2007 09:31:20 +0000 Subject: [PATCH] Sandbox: Fix a security hole so that switcher sets real user ID, saved set_user-ID, real group ID and saved set_group-ID svn path=/trunk/boinc/; revision=13734 --- checkin_notes | 15 +++++++++++++++ client/check_security.C | 6 +++--- client/switcher.C | 29 +++++++++++++++++++++++++++++ clientgui/mac/SetupSecurity.cpp | 14 +++++++------- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/checkin_notes b/checkin_notes index 9c72093a52..04087bc864 100755 --- a/checkin_notes +++ b/checkin_notes @@ -8982,3 +8982,18 @@ David 1 Oct 2007 team_quit_action.php validate_email_addr.php white.css + +Charlie 1 Oct 2007 + Sandbox: Fix a security hole so that switcher sets real user ID, + saved set_user-ID, real group ID and saved set_group-ID for process + in addition to effective user ID and effective group ID. This + prevents a project application from accidentally or maliciously + setting the effective user or group ID back to those of the user + who launched BOINC. + + client/ + check_security.C + switcher.C + clientgui/ + mac/ + SetupSecurity.cpp diff --git a/client/check_security.C b/client/check_security.C index d36f051f4e..801a6f2d06 100644 --- a/client/check_security.C +++ b/client/check_security.C @@ -418,13 +418,13 @@ int use_sandbox, int isManager if (retval) return -1037; - if (sbuf.st_gid != boinc_project_gid) + if (sbuf.st_gid != boinc_master_gid) return -1038; - if (sbuf.st_uid != boinc_project_uid) + if (sbuf.st_uid != 0) // root return -1039; - if ((sbuf.st_mode & 07777) != 06551) + if ((sbuf.st_mode & 07777) != 04050) return -1040; strlcpy(full_path, dir_path, sizeof(dir_path)); diff --git a/client/switcher.C b/client/switcher.C index 1d6421fb5a..0f2b25b28b 100644 --- a/client/switcher.C +++ b/client/switcher.C @@ -25,10 +25,20 @@ #include #include +#include #include #include // for MAXPATHLEN +#include // getpwuid +#include int main(int argc, char** argv) { + int i, j; + passwd *pw; + group *grp; + char user_name[256], group_name[256]; + + strlcpy(user_name, "boinc_project", sizeof(user_name)); + strlcpy(group_name, "boinc_project", sizeof(group_name)); #if 0 // For debugging only char current_dir[MAXPATHLEN]; @@ -42,6 +52,25 @@ int main(int argc, char** argv) { fflush(stderr); #endif +#if 0 // For debugging only + // Allow debugging without running as user or group boinc_project + pw = getpwuid(getuid()); + if (pw) strlcpy(user_name, pw->pw_name, sizeof(user_name)); + grp = getgrgid(getgid()); + if (grp) strlcpy(group_name, grp->gr_gid, sizeof(group_name)); + +#endif + + // We are running setuid root, so setgid() sets real group ID, + // effective group ID and saved set_group-ID for this process + grp = getgrnam(group_name); + if (grp) setgid(grp->gr_gid); + + // We are running setuid root, so setuid() sets real user ID, + // effective user ID and saved set_user-ID for this process + pw = getpwnam(user_name); + if (pw) setuid(pw->pw_uid); + execv(argv[1], argv+2); // If we got here execv failed diff --git a/clientgui/mac/SetupSecurity.cpp b/clientgui/mac/SetupSecurity.cpp index a6b2144813..49d895500c 100644 --- a/clientgui/mac/SetupSecurity.cpp +++ b/clientgui/mac/SetupSecurity.cpp @@ -23,7 +23,7 @@ #include #include // getgrname, getgrgid -#include // getpwname, getpwuid, getuid +#include // getpwnam, getpwuid, getuid #include // usleep #include // for MAXPATHLEN #include @@ -452,17 +452,17 @@ int SetBOINCDataOwnersGroupsAndPermissions() { result = FSPathMakeRef((StringPtr)fullpath, &ref, &isDirectory); if ((result == noErr) && (! isDirectory)) { // Set owner and group of switcher application - sprintf(buf1, "%s:%s", boinc_project_user_name, boinc_project_group_name); - // chown boinc_project:boinc_project "/Library/Applications/BOINC Data/switcher/switcher" + sprintf(buf1, "root:%s", boinc_master_group_name); + // chown root:boinc_master "/Library/Applications/BOINC Data/switcher/switcher" err = DoPrivilegedExec(chownPath, buf1, fullpath, NULL, NULL, NULL); if (err) return err; // Set permissions of switcher application - // chmod u=rsx,g=rsx,o=x "/Library/Applications/BOINC Data/switcher/switcher" - // 06551 = S_ISUID | S_ISGID | S_IRUSR | S_IXUSR | S_IRGRP | S_IXGRP | S_IXOTH - // setuid-on-execution, setgid-on-execution plus read and execute permission for user and group, execute only for others - err = DoPrivilegedExec(chmodPath, "u=rsx,g=rsx,o=x", fullpath, NULL, NULL, NULL); + // chmod u=s,g=rsx,o= "/Library/Applications/BOINC Data/switcher/switcher" + // 04050 = S_ISUID | S_IRGRP | S_IXGRP + // setuid-on-execution, setgid-on-execution plus read and execute permission for group boinc_master only + err = DoPrivilegedExec(chmodPath, "u=s,g=rx,o=", fullpath, NULL, NULL, NULL); if (err) return err; } // switcher application