changeset 1156:faf7117c4489 draft

Fix some issues raised (albeit indirectly) by Isaac Dunham. POLL_IN defined as a constant by some libc. Factor out login.c's change_identity() to xwrap.c as xsetuser(). Replace xsetuid() with xsetuser() Put a space between argument globals and non-argument globals. TT starts zeroed, don't need to re-zero entries in it. STDIN_FILENO has been 0 since 1969, even DOS copied that. Just say 0. Added an xchroot() using xchdir() to lib/xwrap.c. Remove endgrent() call until somebody can explain why it was there.
author Rob Landley <rob@landley.net>
date Mon, 23 Dec 2013 06:49:38 -0600
parents 63f8c7fa94d7
children f10898e637ea
files lib/lib.h lib/xwrap.c main.c toys/lsb/su.c toys/other/login.c toys/pending/tftpd.c
diffstat 6 files changed, 31 insertions(+), 36 deletions(-) [+]
line wrap: on
line diff
--- a/lib/lib.h	Sun Dec 22 20:15:54 2013 -0600
+++ b/lib/lib.h	Mon Dec 23 06:49:38 2013 -0600
@@ -109,11 +109,12 @@
 char *xabspath(char *path, int exact);
 char *xrealpath(char *path);
 void xchdir(char *path);
+void xchroot(char *path);
 void xmkpath(char *path, int mode);
-void xsetuid(uid_t uid);
 struct passwd *xgetpwuid(uid_t uid);
 struct group *xgetgrgid(gid_t gid);
 struct passwd *xgetpwnam(char *name);
+void xsetuser(struct passwd *pwd);
 char *xreadlink(char *name);
 long xparsetime(char *arg, long units, long *fraction);
 void xpidfile(char *name);
--- a/lib/xwrap.c	Sun Dec 22 20:15:54 2013 -0600
+++ b/lib/xwrap.c	Mon Dec 23 06:49:38 2013 -0600
@@ -363,6 +363,12 @@
   if (chdir(path)) error_exit("chdir '%s'", path);
 }
 
+void xchroot(char *path)
+{
+  if (chroot(path)) error_exit("chroot '%s'", path);
+  xchdir("/");
+}
+
 // Ensure entire path exists.
 // If mode != -1 set permissions on newly created dirs.
 // Requires that path string be writable (for temporary null terminators).
@@ -391,14 +397,6 @@
   }
 }
 
-// setuid() can fail (for example, too many processes belonging to that user),
-// which opens a security hole if the process continues as the original user.
-
-void xsetuid(uid_t uid)
-{
-  if (setuid(uid)) perror_exit("xsetuid");
-}
-
 struct passwd *xgetpwuid(uid_t uid)
 {
   struct passwd *pwd = getpwuid(uid);
@@ -420,6 +418,15 @@
   return up;
 }
 
+// setuid() can fail (for example, too many processes belonging to that user),
+// which opens a security hole if the process continues as the original user.
+
+void xsetuser(struct passwd *pwd)
+{
+  if (initgroups(pwd->pw_name, pwd->pw_gid) || setgid(pwd->pw_uid)
+      || setuid(pwd->pw_uid)) perror_exit("xsetuser '%s'", pwd->pw_name);
+}
+
 // This can return null (meaning file not found).  It just won't return null
 // for memory allocation reasons.
 char *xreadlink(char *name)
--- a/main.c	Sun Dec 22 20:15:54 2013 -0600
+++ b/main.c	Mon Dec 23 06:49:38 2013 -0600
@@ -92,7 +92,8 @@
     uid_t uid = getuid(), euid = geteuid();
 
     if (!(which->flags & TOYFLAG_STAYROOT)) {
-      if (uid != euid) xsetuid(euid=uid); // drop root
+      if (uid != euid)
+        if (!setuid(euid=uid)) perror_exit("setuid"); // drop root
     } else if (CFG_TOYBOX_DEBUG && uid && which != toy_list)
       error_msg("Not installed suid root");
 
--- a/toys/lsb/su.c	Sun Dec 22 20:15:54 2013 -0600
+++ b/toys/lsb/su.c	Mon Dec 23 06:49:38 2013 -0600
@@ -60,7 +60,7 @@
   if (!passhash || strcmp(passhash, shp->sp_pwdp)) goto deny;
 
   up = xgetpwnam(name);
-  xsetuid(up->pw_uid);
+  xsetuser(up);
 
   argv = argu = xmalloc(sizeof(char *)*(toys.optc + 4));
   *(argv++) = TT.s ? TT.s : up->pw_shell;
--- a/toys/other/login.c	Sun Dec 22 20:15:54 2013 -0600
+++ b/toys/other/login.c	Mon Dec 23 06:49:38 2013 -0600
@@ -14,6 +14,7 @@
     usage: login [-p] [-h host] [[-f] username]
 
     Establish a new session with the system.
+
     -p	Preserve environment
     -h	The name of the remote host for this login
     -f	Do not perform authentication
@@ -109,15 +110,6 @@
   fflush(stdout);
 }
 
-int change_identity(const struct passwd *pwd)
-{
-  if (initgroups(pwd->pw_name,pwd->pw_gid)) return 1;
-  if (setgid(pwd->pw_uid)) return 1;
-  if (setuid(pwd->pw_uid)) return 1;
-
-  return 0;
-}
-
 void spawn_shell(const char *shell)
 {
   const char * exec_name = strrchr(shell,'/');
@@ -214,7 +206,7 @@
 
   if (pwd->pw_uid) handle_nologin();
 
-  if (change_identity(pwd)) error_exit("Failed to change identity");
+  xsetuser(pwd);
 
   setup_environment(pwd, !(toys.optflags & FLAG_p));
 
--- a/toys/pending/tftpd.c	Sun Dec 22 20:15:54 2013 -0600
+++ b/toys/pending/tftpd.c	Mon Dec 23 06:49:38 2013 -0600
@@ -20,11 +20,13 @@
     -u	Access files as USER
     -l	Log to syslog (inetd mode requires this)
 */
+
 #define FOR_tftpd
 #include "toys.h"
 
 GLOBALS(
   char *user;
+
   long sfd;
   struct passwd *pw;
 )
@@ -97,10 +99,7 @@
 
   pollfds[0].fd = TT.sfd;
   // initialize groups, setgid and setuid
-  if (TT.pw) {
-    if (change_identity(TT.pw)) perror_exit("Failed to change identity");
-    endgrent();
-  }
+  if (TT.pw) xsetuser(TT.pw);
 
   if (opcode == TFTPD_OP_RRQ) fd = open(file, O_RDONLY, 0666);
   else fd = open(file, ((toys.optflags & FLAG_c) ?
@@ -163,11 +162,11 @@
     // if "block size < 512", send ACK and exit.
     if ((pktopcode == TFTPD_OP_ACK) && done) break;
 
-POLL_IN:
+POLL_INPUT:
     pollfds[0].events = POLLIN;
     pollfds[0].fd = TT.sfd;
     poll_ret = poll(pollfds, 1, timeout);
-    if (poll_ret < 0 && (errno == EINTR || errno == ENOMEM)) goto POLL_IN;
+    if (poll_ret < 0 && (errno == EINTR || errno == ENOMEM)) goto POLL_INPUT;
     if (!poll_ret) {
       if (!--retry_count) {
         error_msg("timeout");
@@ -181,7 +180,7 @@
         send_errpkt(dstaddr, socklen, "read-error");
         break;
       }
-      if (len < 4) goto POLL_IN;
+      if (len < 4) goto POLL_INPUT;
     } else {
       perror_msg("poll");
       break;
@@ -224,7 +223,7 @@
       }
       continue;
     }
-    goto POLL_IN;
+    goto POLL_INPUT;
   } // end of loop
 
 CLEAN_APP:
@@ -242,19 +241,14 @@
   socklen_t socklen = sizeof(struct sockaddr_storage);
   char *buf = toybuf;
 
-  TT.pw = NULL;
   memset(&srcaddr, 0, sizeof(srcaddr));
-  if (getsockname(STDIN_FILENO, (struct sockaddr*)&srcaddr, &socklen)) {
+  if (getsockname(0, (struct sockaddr *)&srcaddr, &socklen)) {
     toys.exithelp = 1;
     error_exit(NULL);
   }
 
   if (TT.user) TT.pw = xgetpwnam(TT.user);
-  if (*toys.optargs) {
-    if (chroot(*toys.optargs))
-      perror_exit("can't change root directory to '%s'", *toys.optargs);
-    if (chdir("/")) perror_exit("can't change directory to '/'");
-  }
+  if (*toys.optargs) xchroot(*toys.optargs);
 
   recvmsg_len = recvfrom(STDIN_FILENO, toybuf, TFTPD_BLKSIZE, 0,
       (struct sockaddr*)&dstaddr, &socklen);