From owner-svn-src-all@freebsd.org Sat Mar 10 01:35:27 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id E2AF8F3A032; Sat, 10 Mar 2018 01:35:26 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 910E279197; Sat, 10 Mar 2018 01:35:26 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 8AE4826EDC; Sat, 10 Mar 2018 01:35:26 +0000 (UTC) (envelope-from asomers@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w2A1ZQ5u053987; Sat, 10 Mar 2018 01:35:26 GMT (envelope-from asomers@FreeBSD.org) Received: (from asomers@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w2A1ZQTt053985; Sat, 10 Mar 2018 01:35:26 GMT (envelope-from asomers@FreeBSD.org) Message-Id: <201803100135.w2A1ZQTt053985@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: asomers set sender to asomers@FreeBSD.org using -f From: Alan Somers Date: Sat, 10 Mar 2018 01:35:26 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r330718 - in head/libexec/tftpd: . tests X-SVN-Group: head X-SVN-Commit-Author: asomers X-SVN-Commit-Paths: in head/libexec/tftpd: . tests X-SVN-Commit-Revision: 330718 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 10 Mar 2018 01:35:27 -0000 Author: asomers Date: Sat Mar 10 01:35:26 2018 New Revision: 330718 URL: https://svnweb.freebsd.org/changeset/base/330718 Log: 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 MFC after: 3 weeks Modified: head/libexec/tftpd/tests/functional.c head/libexec/tftpd/tftpd.c Modified: head/libexec/tftpd/tests/functional.c ============================================================================== --- head/libexec/tftpd/tests/functional.c Sat Mar 10 01:28:55 2018 (r330717) +++ head/libexec/tftpd/tests/functional.c Sat Mar 10 01:35:26 2018 (r330718) @@ -350,6 +350,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); } @@ -714,7 +718,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; @@ -782,7 +786,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); @@ -831,7 +835,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: head/libexec/tftpd/tftpd.c ============================================================================== --- head/libexec/tftpd/tftpd.c Sat Mar 10 01:28:55 2018 (r330717) +++ head/libexec/tftpd/tftpd.c Sat Mar 10 01:35:26 2018 (r330718) @@ -743,8 +743,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; } @@ -752,6 +756,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); }