From afba5b8efdf1bac2c02ca787840a2be053c800f7 Mon Sep 17 00:00:00 2001 From: Rob Landley Date: Mon, 23 Dec 2013 06:49:38 -0600 Subject: 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. --- lib/lib.h | 3 ++- lib/xwrap.c | 23 +++++++++++++++-------- main.c | 3 ++- toys/lsb/su.c | 2 +- toys/other/login.c | 12 ++---------- toys/pending/tftpd.c | 24 +++++++++--------------- 6 files changed, 31 insertions(+), 36 deletions(-) diff --git a/lib/lib.h b/lib/lib.h index 3d326499..c46aacaa 100644 --- a/lib/lib.h +++ b/lib/lib.h @@ -109,11 +109,12 @@ void xstat(char *path, struct stat *st); 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); diff --git a/lib/xwrap.c b/lib/xwrap.c index c0c8a44b..08a93ddf 100644 --- a/lib/xwrap.c +++ b/lib/xwrap.c @@ -363,6 +363,12 @@ void xchdir(char *path) 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 @@ void xmkpath(char *path, int mode) } } -// 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 @@ struct passwd *xgetpwnam(char *name) 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) diff --git a/main.c b/main.c index 7355f0a2..8fe8e058 100644 --- a/main.c +++ b/main.c @@ -92,7 +92,8 @@ void toy_init(struct toy_list *which, char *argv[]) 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"); diff --git a/toys/lsb/su.c b/toys/lsb/su.c index 52d20244..04b0dc1d 100644 --- a/toys/lsb/su.c +++ b/toys/lsb/su.c @@ -60,7 +60,7 @@ void su_main() 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; diff --git a/toys/other/login.c b/toys/other/login.c index 54190d0f..100c665a 100644 --- a/toys/other/login.c +++ b/toys/other/login.c @@ -14,6 +14,7 @@ config LOGIN 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 @@ void handle_motd(void) 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 @@ query_pass: 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)); diff --git a/toys/pending/tftpd.c b/toys/pending/tftpd.c index ea8d3eac..41a1cc72 100644 --- a/toys/pending/tftpd.c +++ b/toys/pending/tftpd.c @@ -20,11 +20,13 @@ config TFTPD -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 @@ static void do_action(struct sockaddr *srcaddr, struct sockaddr *dstaddr, 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 @@ RETRY_SEND: // 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 @@ POLL_IN: 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 @@ POLL_IN: } continue; } - goto POLL_IN; + goto POLL_INPUT; } // end of loop CLEAN_APP: @@ -242,19 +241,14 @@ void tftpd_main(void) 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); -- cgit v1.2.3