[net.bugs.4bsd] SECURITY HOLE IN tftpd

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