From nobody Sun Jul 21 22:57:55 2024 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4WRzMl4W8Vz5Qjb4; Sun, 21 Jul 2024 22:57:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4WRzMl2mwBz4VpV; Sun, 21 Jul 2024 22:57:55 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1721602675; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=xBRLuOve5wBgQ6nc8aUoxmrOKNb9YL2Dlg43BTFvPc4=; b=gQiZGEPutjQmMvbzQxmKqe+PUxUk+xRRC2Dxeg3l/8JbqXsqY6E1ScuiAMWYxRg1531pq2 8AoCU0mWg8BRkRwXbZAMHfOJfXVDg4nHN3kFRE821lUbQ+dxETKLV5WqAl099GzxJD6JUj AC4QUqyJO13MrkbulXi8MVS93biDeiUiC+DoIa2U/OUm0Bz3doaq68q4MhahaZqfNTtNeO lA2f4AMklIHd3YDco4VhbahnWPpNJr4bwDKU85cow3+gEY0SAkTvU2bNQJs0jaAxzFB0Rh RO2N23AMdUUbsr3UsqyaFaaRldMg61S0pJKouKXjZtop1Cv/CNHhziSxg9nVKw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1721602675; a=rsa-sha256; cv=none; b=m5GlvYKcxGjlDdzVbDtX/vuiMhayjQKOMkZUeKG6vjt5MK3NZ2ANiVaEgIgP54BJIoHAd/ ftdG+p9HVsdBKfv/VNdL84sXws0sCIm5+WdDT2AFbbxjv1ud7qfxRpKlBIjTBPZU/q3ZbB eCoAxtiT4fIB+cjy8osNoDkfWivYr9CHPUES66o2TpEBLBmyuDtW0CJgZ+Gn2B5iovcG/8 zpdjnb42tUGdI7XXdd0G5dkEwVDcaNGirEkRWBZqJo/H4vaYSy3A7zoBL3AXqRtWZOQDlO tnavAm5k13k4E36iyA/maM8K+uxvP9X9yPoex4gdJsxi4dqkgBamLYifYVHP4w== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1721602675; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=xBRLuOve5wBgQ6nc8aUoxmrOKNb9YL2Dlg43BTFvPc4=; b=cdvVHQ+27YrNMEPF8Egrm4CDClUdqMBsZCENjJE6bnPjK9bm1Qrg2/GqCLxMqCO/voioZC TdNY0nggPKb1CZV75ma9FLgc1fazQoUIJDvRKuo+LwpzYM4xtKK3ooMx6HadIA3p8rFG9l V6kbRe6O4j/APjikxo/qxE+CnQAJmumN6EwhqTRK+6u7ZXk5cPaDekMfAA7yTwiYePFxfY lpi5gMnZjCFyPjRZGxc4u+xQb7c0Gn2fnHa6Yg/Orq/6qp3YseY6FF15L4qFNxVNhu65Uf BYy5weVQA/BFRVP1Wf1bnCc9WnIN2/6lzOZbOqJpqM8AAAlN7K9LfeLIiQ2sSg== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4WRzMl2FnNzKfB; Sun, 21 Jul 2024 22:57:55 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 46LMvtLR004416; Sun, 21 Jul 2024 22:57:55 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 46LMvtLg004413; Sun, 21 Jul 2024 22:57:55 GMT (envelope-from git) Date: Sun, 21 Jul 2024 22:57:55 GMT Message-Id: <202407212257.46LMvtLg004413@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Rick Macklem Subject: git: 026cdaa3b3a9 - main - nfscl: Scan readdir reply filenames for invalid characters List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: rmacklem X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 026cdaa3b3a92574d9ac3155216e5cc0b0bd4c51 Auto-Submitted: auto-generated The branch main has been updated by rmacklem: URL: https://cgit.FreeBSD.org/src/commit/?id=026cdaa3b3a92574d9ac3155216e5cc0b0bd4c51 commit 026cdaa3b3a92574d9ac3155216e5cc0b0bd4c51 Author: Rick Macklem AuthorDate: 2024-07-21 22:56:16 +0000 Commit: Rick Macklem CommitDate: 2024-07-21 22:56:16 +0000 nfscl: Scan readdir reply filenames for invalid characters The NFS RFCs are pretty loose with respect to what characters can be in a filename returned by a Readdir. However, FreeBSD, as a POSIX system will not handle imbedded '/' or nul characters in file names. Also, for NFSv4, the file names "." and ".." are handcrafted on the client and should not be returned by a NFSv4 server. This patch scans for the above in filenames returned by Readdir and ignores any entry returned by Readdir which has them in it. Because an imbedded nul would be a string terminator, it was not possible to code this check efficiently using string(3) functions. Reported by: Apple Security Engineering and Architecture (SEAR) MFC after: 1 week --- sys/fs/nfsclient/nfs_clrpcops.c | 137 ++++++++++++++++++++++++++++++++-------- 1 file changed, 110 insertions(+), 27 deletions(-) diff --git a/sys/fs/nfsclient/nfs_clrpcops.c b/sys/fs/nfsclient/nfs_clrpcops.c index 13bdc74655dd..8947b608b743 100644 --- a/sys/fs/nfsclient/nfs_clrpcops.c +++ b/sys/fs/nfsclient/nfs_clrpcops.c @@ -142,6 +142,7 @@ static int nfsrpc_createv4(vnode_t , char *, int, struct vattr *, nfsquad_t, int, struct nfsclowner *, struct nfscldeleg **, struct ucred *, NFSPROC_T *, struct nfsvattr *, struct nfsvattr *, struct nfsfh **, int *, int *, int *); +static bool nfscl_invalidfname(bool, char *, int); static int nfsrpc_locku(struct nfsrv_descript *, struct nfsmount *, struct nfscllockowner *, u_int64_t, u_int64_t, u_int32_t, struct ucred *, NFSPROC_T *, int); @@ -3321,6 +3322,31 @@ nfsrpc_rmdir(vnode_t dvp, char *name, int namelen, struct ucred *cred, return (error); } +/* + * Check to make sure the file name in a Readdir reply is valid. + */ +static bool +nfscl_invalidfname(bool is_v4, char *name, int len) +{ + int i; + char *cp; + + if (is_v4 && ((len == 1 && name[0] == '.') || + (len == 2 && name[0] == '.' && name[1] == '.'))) { + printf("Readdir NFSv4 reply has dot or dotdot in it\n"); + return (true); + } + cp = name; + for (i = 0; i < len; i++, cp++) { + if (*cp == '/' || *cp == '\0') { + printf("Readdir reply file name had imbedded / or nul" + " byte\n"); + return (true); + } + } + return (false); +} + /* * Readdir rpc. * Always returns with either uio_resid unchanged, if you are at the @@ -3373,6 +3399,8 @@ nfsrpc_readdir(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep, KASSERT(uiop->uio_iovcnt == 1 && (uiop->uio_resid & (DIRBLKSIZ - 1)) == 0, ("nfs readdirrpc bad uio")); + KASSERT(uiop->uio_segflg == UIO_SYSSPACE, + ("nfsrpc_readdir: uio userspace")); ncookie.lval[0] = ncookie.lval[1] = 0; /* * There is no point in reading a lot more than uio_resid, however @@ -3630,6 +3658,17 @@ nfsrpc_readdir(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep, uiop->uio_resid) bigenough = 0; if (bigenough) { + struct iovec saviov; + off_t savoff; + ssize_t savresid; + int savblksiz; + + saviov.iov_base = uiop->uio_iov->iov_base; + saviov.iov_len = uiop->uio_iov->iov_len; + savoff = uiop->uio_offset; + savresid = uiop->uio_resid; + savblksiz = blksiz; + dp = (struct dirent *)uiop->uio_iov->iov_base; dp->d_pad0 = dp->d_pad1 = 0; dp->d_off = 0; @@ -3645,20 +3684,35 @@ nfsrpc_readdir(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep, uiop->uio_iov->iov_base = (char *)uiop->uio_iov->iov_base + DIRHDSIZ; uiop->uio_iov->iov_len -= DIRHDSIZ; + cp = uiop->uio_iov->iov_base; error = nfsm_mbufuio(nd, uiop, len); if (error) goto nfsmout; - cp = uiop->uio_iov->iov_base; - tlen -= len; - NFSBZERO(cp, tlen); - cp += tlen; /* points to cookie storage */ - tl2 = (u_int32_t *)cp; - uiop->uio_iov->iov_base = - (char *)uiop->uio_iov->iov_base + tlen + - NFSX_HYPER; - uiop->uio_iov->iov_len -= tlen + NFSX_HYPER; - uiop->uio_resid -= tlen + NFSX_HYPER; - uiop->uio_offset += (tlen + NFSX_HYPER); + /* Check for an invalid file name. */ + if (nfscl_invalidfname( + (nd->nd_flag & ND_NFSV4) != 0, cp, len)) { + /* Skip over this entry. */ + uiop->uio_iov->iov_base = + saviov.iov_base; + uiop->uio_iov->iov_len = + saviov.iov_len; + uiop->uio_offset = savoff; + uiop->uio_resid = savresid; + blksiz = savblksiz; + } else { + cp = uiop->uio_iov->iov_base; + tlen -= len; + NFSBZERO(cp, tlen); + cp += tlen; /* points to cookie store */ + tl2 = (u_int32_t *)cp; + uiop->uio_iov->iov_base = + (char *)uiop->uio_iov->iov_base + + tlen + NFSX_HYPER; + uiop->uio_iov->iov_len -= tlen + + NFSX_HYPER; + uiop->uio_resid -= tlen + NFSX_HYPER; + uiop->uio_offset += (tlen + NFSX_HYPER); + } } else { error = nfsm_advance(nd, NFSM_RNDUP(len), -1); if (error) @@ -3824,6 +3878,8 @@ nfsrpc_readdirplus(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep, KASSERT(uiop->uio_iovcnt == 1 && (uiop->uio_resid & (DIRBLKSIZ - 1)) == 0, ("nfs readdirplusrpc bad uio")); + KASSERT(uiop->uio_segflg == UIO_SYSSPACE, + ("nfsrpc_readdirplus: uio userspace")); ncookie.lval[0] = ncookie.lval[1] = 0; timespecclear(&dctime); *attrflagp = 0; @@ -4059,6 +4115,17 @@ nfsrpc_readdirplus(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep, uiop->uio_resid) bigenough = 0; if (bigenough) { + struct iovec saviov; + off_t savoff; + ssize_t savresid; + int savblksiz; + + saviov.iov_base = uiop->uio_iov->iov_base; + saviov.iov_len = uiop->uio_iov->iov_len; + savoff = uiop->uio_offset; + savresid = uiop->uio_resid; + savblksiz = blksiz; + dp = (struct dirent *)uiop->uio_iov->iov_base; dp->d_pad0 = dp->d_pad1 = 0; dp->d_off = 0; @@ -4077,25 +4144,41 @@ nfsrpc_readdirplus(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep, cnp->cn_nameptr = uiop->uio_iov->iov_base; cnp->cn_namelen = len; NFSCNHASHZERO(cnp); + cp = uiop->uio_iov->iov_base; error = nfsm_mbufuio(nd, uiop, len); if (error) goto nfsmout; - cp = uiop->uio_iov->iov_base; - tlen -= len; - NFSBZERO(cp, tlen); - cp += tlen; /* points to cookie storage */ - tl2 = (u_int32_t *)cp; - if (len == 2 && cnp->cn_nameptr[0] == '.' && - cnp->cn_nameptr[1] == '.') - isdotdot = 1; - else - isdotdot = 0; - uiop->uio_iov->iov_base = - (char *)uiop->uio_iov->iov_base + tlen + - NFSX_HYPER; - uiop->uio_iov->iov_len -= tlen + NFSX_HYPER; - uiop->uio_resid -= tlen + NFSX_HYPER; - uiop->uio_offset += (tlen + NFSX_HYPER); + /* Check for an invalid file name. */ + if (nfscl_invalidfname( + (nd->nd_flag & ND_NFSV4) != 0, cp, len)) { + /* Skip over this entry. */ + uiop->uio_iov->iov_base = + saviov.iov_base; + uiop->uio_iov->iov_len = + saviov.iov_len; + uiop->uio_offset = savoff; + uiop->uio_resid = savresid; + blksiz = savblksiz; + } else { + cp = uiop->uio_iov->iov_base; + tlen -= len; + NFSBZERO(cp, tlen); + cp += tlen; /* points to cookie store */ + tl2 = (u_int32_t *)cp; + if (len == 2 && + cnp->cn_nameptr[0] == '.' && + cnp->cn_nameptr[1] == '.') + isdotdot = 1; + else + isdotdot = 0; + uiop->uio_iov->iov_base = + (char *)uiop->uio_iov->iov_base + + tlen + NFSX_HYPER; + uiop->uio_iov->iov_len -= tlen + + NFSX_HYPER; + uiop->uio_resid -= tlen + NFSX_HYPER; + uiop->uio_offset += (tlen + NFSX_HYPER); + } } else { error = nfsm_advance(nd, NFSM_RNDUP(len), -1); if (error)