svn commit: r337248 - in stable/10/libexec/tftpd: . tests

[Alan Somers]

Author: asomers
Date: Fri Aug  3 14:18:02 2018
New Revision: 337248
URL: https://svnweb.freebsd.org/changeset/base/337248

Log:
  MFC r330718:
  
  tftpd: Verify world-writability for WRQ when using relative paths
  
  tftpd(8) says that files may only be written if they already exist and are
  publicly writable.  tftpd.c verifies that a file is publicly writable if it
  uses an absolute pathname.  However, if the pathname is relative, that check
  is skipped.  Fix it.
  
  Note that this is not a security vulnerability, because the transfer
  ultimately doesn't work unless the file already exists and is owned by user
  nobody.  Also, this bug does not affect the default configuration, because
  the default uses the "-s" option which makes all pathnames absolute.
  
  PR:		226004

Modified:
  stable/10/libexec/tftpd/tests/functional.c
  stable/10/libexec/tftpd/tftpd.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/libexec/tftpd/tests/functional.c
==============================================================================
--- stable/10/libexec/tftpd/tests/functional.c	Fri Aug  3 14:17:11 2018	(r337247)
+++ stable/10/libexec/tftpd/tests/functional.c	Fri Aug  3 14:18:02 2018	(r337248)
@@ -348,6 +348,10 @@ setup(struct sockaddr_storage *to, uint16_t idx)
 		ATF_REQUIRE((client_s = socket(protocol, SOCK_DGRAM, 0)) > 0);
 		break;
 	}
+
+	/* Clear the client's umask.  Test cases will specify exact modes */
+	umask(0000);
+
 	return (client_s);
 }
 
@@ -713,7 +717,7 @@ TFTPD_TC_DEFINE(wrq_dropped_ack,)
 	for (i = 0; i < nitems(contents); i++)
 		contents[i] = i;
 
-	fd = open("medium.txt", O_RDWR | O_CREAT, 0644);
+	fd = open("medium.txt", O_RDWR | O_CREAT, 0666);
 	ATF_REQUIRE(fd >= 0);
 	close(fd);
 
@@ -747,7 +751,7 @@ TFTPD_TC_DEFINE(wrq_dropped_data,)
 	size_t contents_len;
 	char buffer[1024];
 
-	fd = open("small.txt", O_RDWR | O_CREAT, 0644);
+	fd = open("small.txt", O_RDWR | O_CREAT, 0666);
 	ATF_REQUIRE(fd >= 0);
 	close(fd);
 	contents_len = strlen(contents) + 1;
@@ -783,7 +787,7 @@ TFTPD_TC_DEFINE(wrq_duped_data,)
 	for (i = 0; i < nitems(contents); i++)
 		contents[i] = i;
 
-	fd = open("medium.txt", O_RDWR | O_CREAT, 0644);
+	fd = open("medium.txt", O_RDWR | O_CREAT, 0666);
 	ATF_REQUIRE(fd >= 0);
 	close(fd);
 
@@ -833,7 +837,8 @@ TFTPD_TC_DEFINE(wrq_eaccess_world_readable,)
 	close(fd);
 
 	SEND_WRQ("empty.txt", "octet");
-	atf_tc_expect_fail("PR 226004 with relative pathnames, tftpd doesn't validate world writability");
+	atf_tc_expect_fail("PR 225996 tftpd doesn't abort on a WRQ access "
+	    "violation");
 	RECV_ERROR(2, "Access violation");
 }
 

Modified: stable/10/libexec/tftpd/tftpd.c
==============================================================================
--- stable/10/libexec/tftpd/tftpd.c	Fri Aug  3 14:17:11 2018	(r337247)
+++ stable/10/libexec/tftpd/tftpd.c	Fri Aug  3 14:18:02 2018	(r337248)
@@ -741,8 +741,12 @@ validate_access(int peer, char **filep, int mode)
 				dirp->name, filename);
 			if (stat(pathname, &stbuf) == 0 &&
 			    (stbuf.st_mode & S_IFMT) == S_IFREG) {
-				if ((stbuf.st_mode & S_IROTH) != 0) {
-					break;
+				if (mode == RRQ) {
+					if ((stbuf.st_mode & S_IROTH) != 0)
+						break;
+				} else {
+					if ((stbuf.st_mode & S_IWOTH) != 0)
+						break;
 				}
 				err = EACCESS;
 			}
@@ -750,6 +754,8 @@ validate_access(int peer, char **filep, int mode)
 		if (dirp->name != NULL)
 			*filep = filename = pathname;
 		else if (mode == RRQ)
+			return (err);
+		else if (err != ENOTFOUND || !create_new)
 			return (err);
 	}