From 456a9f9905482a27479a30e2fe48b7447402ff43 Mon Sep 17 00:00:00 2001 From: asomers Date: Fri, 3 Aug 2018 14:18:02 +0000 Subject: [PATCH] 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 git-svn-id: svn://svn.freebsd.org/base/stable/10@337248 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f --- libexec/tftpd/tests/functional.c | 13 +++++++++---- libexec/tftpd/tftpd.c | 10 ++++++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/libexec/tftpd/tests/functional.c b/libexec/tftpd/tests/functional.c index af8919968..d67086515 100644 --- a/libexec/tftpd/tests/functional.c +++ b/libexec/tftpd/tests/functional.c @@ -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"); } diff --git a/libexec/tftpd/tftpd.c b/libexec/tftpd/tftpd.c index cd5cd2e01..2b2e54910 100644 --- a/libexec/tftpd/tftpd.c +++ b/libexec/tftpd/tftpd.c @@ -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; } @@ -751,6 +755,8 @@ validate_access(int peer, char **filep, int mode) *filep = filename = pathname; else if (mode == RRQ) return (err); + else if (err != ENOTFOUND || !create_new) + return (err); } /* -- 2.42.0