shannon@sun.uucp (Bill Shannon) (03/08/84)
Subject: tftpd doesn't check file permissions properly Index: etc/tftpd.c 4.2BSD Description: The tftp daemon runs as root and is only supposed to let you access files with public read. However, it only checks the file itself, not the path to the file. Repeat-By: chmod 700 /sys tftp localhost get /sys/sys/tty.c Fix: I fixed it by doing a setgid(-2), setuid(-2) before checking access permissions. It's hard to check the entire path by hand because of symbolic links; you really have to run as someone who will only have public permission to the file. -2/-2 is not guaranteed to be restrictive enough, but it was a quick fix. Perhaps a uid/gid should be reserved for this purpose. Sorry, no diff of the fix. Our tftpd has changed far too much for other reasons for it to be useful.
lee@unmvax.UUCP (09/21/84)
Here is Bill Shannon's original article.. >From shannon@sun.uucp (Bill Shannon) Sat Feb 5 23:28:16 206 >Relay-Version: version B 2.10.1 6/24/83; site unmvax.UUCP >Posting-Version: version B 2.10.1 6/24/83 SMI; site sun.uucp >Path: unmvax!unm-cvax!lanl-a!cmcl2!floyd!harpo!decvax!decwrl!sun!shannon >From: shannon@sun.uucp (Bill Shannon) >Newsgroups: net.bugs.4bsd >Subject: SECURITY HOLE in tftpd >Message-ID: <566@sun.uucp> >Date: Wed, 7-Mar-84 16:36:55 MST >Date-Received: Thu, 8-Mar-84 13:13:09 MST >Organization: Sun Microsystems, Inc. >Lines: 21 > >Subject: tftpd doesn't check file permissions properly >Index: etc/tftpd.c 4.2BSD > >Description: > The tftp daemon runs as root and is only supposed to let you > access files with public read. However, it only checks the > file itself, not the path to the file. >Repeat-By: > chmod 700 /sys > tftp localhost > get /sys/sys/tty.c >Fix: > I fixed it by doing a setgid(-2), setuid(-2) before checking access > permissions. It's hard to check the entire path by hand because > of symbolic links; you really have to run as someone who will only > have public permission to the file. -2/-2 is not guaranteed to be > restrictive enough, but it was a quick fix. Perhaps a uid/gid should > be reserved for this purpose. > > Sorry, no diff of the fix. Our tftpd has changed far too much for > other reasons for it to be useful. > I came up with a different fix that does not require a reset of the UID/GID. Nor does it need a special user id. Just runs as root. RCS file: RCS/tftpd.c,v retrieving revision 1.1 diff -c -r1.1 tftpd.c *** /tmp/,RCSt1021188 Thu Sep 20 16:22:24 1984 --- tftpd.c Thu Sep 20 16:08:43 1984 *************** *** 188,193 int mode; { struct stat stbuf; if (*file != '/') return (EACCESS); --- 188,195 ----- int mode; { struct stat stbuf; + char *ptr; + int sret; if (*file != '/') return (EACCESS); *************** *** 191,196 if (*file != '/') return (EACCESS); if (stat(file, &stbuf) < 0) return (errno == ENOENT ? ENOTFOUND : EACCESS); if (mode == RRQ) { --- 193,215 ----- if (*file != '/') return (EACCESS); + /* Check path first */ + ptr = file; + ptr++; + while (*ptr) { + if (*ptr++ != '/') + continue; + ptr--; + *ptr = NULL; + sret = stat(file, &stbuf); + *ptr++ = '/'; + if (sret < 0) + return (errno == ENOENT ? ENOTFOUND : EACCESS); + if (!((stbuf.st_mode&S_IFMT)&S_IFDIR)) + break; + if ((stbuf.st_mode&(S_IEXEC >> 6)) == 0) + return (EACCESS); + } if (stat(file, &stbuf) < 0) return (errno == ENOENT ? ENOTFOUND : EACCESS); if (mode == RRQ) { -- --Lee (Ward) {ucbvax,convex,gatech,pur-ee}!unmvax!lee
chris@umcp-cs.UUCP (Chris Torek) (09/21/84)
Perhaps the solution to ``who is the user with no permissions'' is to claim that every system should have a login and group name of ``guest'' (not necessarily one that can be used to log in). That is, /etc/passwd might have . . . guest:*:99:99:Guest account:/tmp:/bin/notashell . . . and /etc/group would then have guest:*:99: in it. Then any setuid program that must have no special permissions can use getpwnam and/or getgrnam to set its user and group IDs. Then again, perhaps that's not the solution. (Do I need this? :-)) -- (This page accidently left blank.) In-Real-Life: Chris Torek, Univ of MD Comp Sci (301) 454-7690 UUCP: {seismo,allegra,brl-bmd}!umcp-cs!chris CSNet: chris@umcp-cs ARPA: chris@maryland
larry@utecfa.UUCP (Larry Philps) (09/28/84)
<die creature> Sorry, the "better" fix shown below will not work. >From: lee@unmvax.UUCP >Subject: SECURITY HOLE in tftpd > ... >+ /* Check path first */ >+ ptr = file; >+ ptr++; >+ while (*ptr) { >+ if (*ptr++ != '/') >+ continue; >+ ptr--; >+ *ptr = NULL; >+ sret = stat(file, &stbuf); /*********/ >+ *ptr++ = '/'; >+ if (sret < 0) >+ return (errno == ENOENT ? ENOTFOUND : EACCESS); >+ if (!((stbuf.st_mode&S_IFMT)&S_IFDIR)) >+ break; >+ if ((stbuf.st_mode&(S_IEXEC >> 6)) == 0) >+ return (EACCESS); >+ } Bill Shannon stated that it was hard to do this because of symbolic links, and he was right. The stat done in the middle of the loop (marked above by /******/) executed run as root, and thus if a symbolic link is encountered, all directories/files in the link will be searched as root. For example, # chmod 700 /sys # su guest % cd % ln -s sneaky /sys/sys/ufs_syscalls.c % tftp localhost % get sneaky Will get the file since the stat will only check ./sneaky, and /sys/sys/ufs_syscalls.c. The intervening directories, /sys and /sys/sys, will not be checked. -- Larry Philps Engineering Computing Facility University of Toronto {linus,ihnp4,uw-beaver,floyd,decvax,utzoo}!utcsrgv!utecfa!larry
lee@unmvax.UUCP (09/28/84)
The last "fix" I posted neglected to account for symlinks. I still live in a 4.1 world, I guess.... This one gets the symlinks too. RCS file: RCS/tftpd.c,v retrieving revision 1.1 diff -c -r1.1 tftpd.c *** /tmp/,RCSt1013277 Thu Sep 27 16:40:55 1984 --- tftpd.c Thu Sep 27 16:34:50 1984 *************** *** 188,193 int mode; { struct stat stbuf; if (*file != '/') return (EACCESS); --- 188,200 ----- int mode; { struct stat stbuf; + char *ptr, + *lsptr, + *eptr, + buf[BUFSIZ], + tbuf[BUFSIZ]; + int sret, + cc; (void )chdir("/"); if (*file != '/') *************** *** 189,194 { struct stat stbuf; if (*file != '/') return (EACCESS); if (stat(file, &stbuf) < 0) --- 196,202 ----- int sret, cc; + (void )chdir("/"); if (*file != '/') return (EACCESS); /* Check path first */ *************** *** 191,196 if (*file != '/') return (EACCESS); if (stat(file, &stbuf) < 0) return (errno == ENOENT ? ENOTFOUND : EACCESS); if (mode == RRQ) { --- 199,245 ----- (void )chdir("/"); if (*file != '/') return (EACCESS); + /* Check path first */ + eptr = file + strlen(file); + lsptr = ptr = file; + ptr++; + while (ptr < eptr) { + if (*ptr++ != '/' && *ptr) + continue; + if (*ptr) { + ptr--; + *ptr = NULL; + } + sret = lstat(file, &stbuf); + if (ptr != eptr) + *ptr++ = '/'; + else + ptr++; + if (sret < 0) + return (errno == ENOENT ? ENOTFOUND : EACCESS); + if ((stbuf.st_mode&S_IFMT) == S_IFDIR) { + if((stbuf.st_mode&(S_IEXEC >> 6)) == 0) + return (EACCESS); + } else if ((stbuf.st_mode&S_IFMT) == S_IFLNK) { + *--ptr = NULL; + cc = readlink(file, tbuf, sizeof(tbuf)); + if (cc < 0) + return(errno); + tbuf[cc] = NULL; + strncat(tbuf, ptr, sizeof(tbuf) - 1 - cc); + if (tbuf[0] != '/') { + *lsptr = NULL; + (void )chdir(file); + ptr = buf; + } else + ptr = &buf[1]; + strcpy(buf, tbuf); + file = buf; + eptr = file + strlen(file); + } else + break; + lsptr = ptr - 1; + } if (stat(file, &stbuf) < 0) return (errno == ENOENT ? ENOTFOUND : EACCESS); if (mode == RRQ) { -- --Lee (Ward) {ucbvax,convex,gatech,pur-ee}!unmvax!lee