X-Git-Url: http://git.hungrycats.org/cgi-bin/gitweb.cgi?a=blobdiff_plain;f=driver%2Fsetuid.c;h=343dcf097dba0bb5b0b8a234492420a6ddeb6145;hb=14627f4038ada5d11456f3770090f3c39740d70f;hp=399f7df05ebc391402591f7fe9f1d0c277010ba7;hpb=2a991a811de4c7b22f812682b267b616a809fd9a;p=xscreensaver diff --git a/driver/setuid.c b/driver/setuid.c index 399f7df0..343dcf09 100644 --- a/driver/setuid.c +++ b/driver/setuid.c @@ -14,8 +14,6 @@ # include "config.h" #endif -#ifndef NO_SETUID /* whole file */ - #include /* not used for much... */ /* This file doesn't need the Xt headers, so stub these types out... */ @@ -35,9 +33,8 @@ #include /* for getpwnam() and struct passwd */ #include /* for getgrgid() and struct group */ - static const char * -uid_gid_string(uid_t uid, gid_t gid) +uid_gid_string (uid_t uid, gid_t gid) { static char buf[255]; struct passwd *p = 0; @@ -78,12 +75,12 @@ describe_uids (saver_info *si, FILE *out) static int -set_ids_by_name (struct passwd *p, struct group *g, char **message_ret) +set_ids_by_number (uid_t uid, gid_t gid, char **message_ret) { int uid_errno = 0; int gid_errno = 0; - uid_t uid = p->pw_uid; - gid_t gid = g->gr_gid; + struct passwd *p = getpwuid (uid); + struct group *g = getgrgid (gid); if (message_ret) *message_ret = 0; @@ -93,7 +90,8 @@ set_ids_by_name (struct passwd *p, struct group *g, char **message_ret) -1, then that would be Really Bad. Rumor further has it that such systems really ought to be using -2 for "nobody", since that works. So, if we get a uid (or gid, for good measure) of -1, switch to -2 - instead. + instead. Note that this must be done after we've looked up the + user/group names with getpwuid(-1) and/or getgrgid(-1). */ if (gid == (gid_t) -1) gid = (gid_t) -2; if (uid == (uid_t) -1) uid = (uid_t) -2; @@ -110,7 +108,8 @@ set_ids_by_name (struct passwd *p, struct group *g, char **message_ret) { static char buf [1024]; sprintf (buf, "changed uid/gid to %s/%s (%ld/%ld).", - p->pw_name, (g ? g->gr_name : "???"), + (p && p->pw_name ? p->pw_name : "???"), + (g && g->gr_name ? g->gr_name : "???"), (long) uid, (long) gid); if (message_ret) *message_ret = buf; @@ -123,7 +122,7 @@ set_ids_by_name (struct passwd *p, struct group *g, char **message_ret) { sprintf (buf, "%s: couldn't set gid to %s (%ld)", blurb(), - (g ? g->gr_name : "???"), + (g && g->gr_name ? g->gr_name : "???"), (long) gid); if (gid_errno == -1) fprintf(stderr, "%s: unknown error\n", buf); @@ -135,7 +134,7 @@ set_ids_by_name (struct passwd *p, struct group *g, char **message_ret) { sprintf (buf, "%s: couldn't set uid to %s (%ld)", blurb(), - (p ? p->pw_name : "???"), + (p && p->pw_name ? p->pw_name : "???"), (long) uid); if (uid_errno == -1) fprintf(stderr, "%s: unknown error\n", buf); @@ -147,49 +146,17 @@ set_ids_by_name (struct passwd *p, struct group *g, char **message_ret) } } -static int -set_ids_by_number (uid_t uid, gid_t gid, char **message_ret) -{ - struct passwd *p; - struct group *g; - - errno = 0; - p = getpwuid (uid); - if (!p) - { - char buf [1024]; - sprintf (buf, "%s: error looking up name of user %d", blurb(), - (long) uid); - if (errno) - perror (buf); - else - fprintf (stderr, "%s: unknown error.\n", buf); - return -1; - } - - errno = 0; - g = getgrgid (gid); - if (!g) - { - char buf [1024]; - sprintf (buf, "%s: error looking up name of group %d", blurb(), - (long) gid); - if (errno) - perror (buf); - else - fprintf (stderr, "%s: unknown error.\n", buf); - return -1; - } - - return set_ids_by_name (p, g, message_ret); -} - /* If we've been run as setuid or setgid to someone else (most likely root) turn off the extra permissions so that random user-specified programs don't get special privileges. (On some systems it is necessary to install this program as setuid root in order to read the passwd file to implement lock-mode.) + + *** WARNING: DO NOT DISABLE ANY OF THE FOLLOWING CODE! + If you do so, you will open a security hole. See the sections + of the xscreensaver manual titled "LOCKING AND ROOT LOGINS", + and "USING XDM". */ void hack_uid (saver_info *si) @@ -211,9 +178,15 @@ hack_uid (saver_info *si) saver_exit (si, 1, 0); } + /* Locking can't work when running as root, because we have no way of knowing what the user id of the logged in user is (so we don't know whose password to prompt for.) + + *** WARNING: DO NOT DISABLE THIS CODE! + If you do so, you will open a security hole. See the sections + of the xscreensaver manual titled "LOCKING AND ROOT LOGINS", + and "USING XDM". */ if (getuid() == (uid_t) 0) { @@ -221,11 +194,18 @@ hack_uid (saver_info *si) si->nolock_reason = "running as root"; } + /* If we're running as root, switch to a safer user. This is above and beyond the fact that we've disabling locking, above -- the theory is that running graphics demos as root is just always a stupid thing to do, since they have probably never been security reviewed and are more likely to be buggy than just about any other kind of program. + (And that assumes non-malicious code. There are also attacks here.) + + *** WARNING: DO NOT DISABLE THIS CODE! + If you do so, you will open a security hole. See the sections + of the xscreensaver manual titled "LOCKING AND ROOT LOGINS", + and "USING XDM". */ if (getuid() == (uid_t) 0) { @@ -253,6 +233,11 @@ hack_uid (saver_info *si) and not normal end-users) then disable locking. If it was possible, switching to "nobody" would be the thing to do, but only root itself has the privs to do that. + + *** WARNING: DO NOT DISABLE THIS CODE! + If you do so, you will open a security hole. See the sections + of the xscreensaver manual titled "LOCKING AND ROOT LOGINS", + and "USING XDM". */ { uid_t uid = getuid (); /* get it again */ @@ -287,10 +272,3 @@ hack_uid (saver_info *si) } } } - -#else /* !NO_SETUID */ - -void hack_uid (saver_info *si) { } -void describe_uids (saver_info *si, FILE *out) { } - -#endif /* NO_SETUID */