From nobody Wed Aug 07 13:44:21 2024 X-Original-To: dev-commits-src-all@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 4WfBH94J1Cz5S8r2; Wed, 07 Aug 2024 13:44:21 +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 4WfBH93hBCz4hWr; Wed, 7 Aug 2024 13:44:21 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1723038261; 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=rcFa05g/quTSmsQac8GoTI+wAFqajW5Wo9WxRGL/oxQ=; b=Ru7+7/lymlsABSabyeaBTjdTuEx5sVy4TyCpQo9Kxe799QJDvo37gLpPM0QGqDYM9pTXLL RRVTwlPAojzS/T7OGe/uRip5TPcpp8j02MaX8BHOCNO6dMsHROCwe5dc6JTxm0gmJ1of3G qNAy4Ww9QWEDgkSMfXFlKlToMymaK7FsbI8eMZYJ4gHNK/A29y0KE6tLpk/FVMibAzbxfN z3gIUgR4bn6pUQCcu+QuNNzwM5kL8SjcgeDsdVX4rnI9GdwdA1rOsqkNQlzt+mOMkidIZ2 XFZWpCHX0ci+2Cl4LBO7hUOCAIhQlH+YGQNWqWKozXwr8BacYOHfkbNaiVKyBQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1723038261; a=rsa-sha256; cv=none; b=wJd4mQPRY0n45MhiOcNmXyquj6xGo+ytiNgckjzflBJRs496et2z+kRby0ydzhwqKq0hgx efxbUFCgM1SUcjGUziixNc7cOH/M+jVw4Md1JNjX2UVS2THRmbjPgHNvX0JgJ/94odhcf5 hz0+2nshK4DH/lXKUizbY/F02hm98DlVE3W1xwBdclaQrdhrXJE4k6jYVEN3YW5jombiEv o5RMmNSSAdiCMCd0rco5lOjmI7bXfw5mzpTIgcDOHOd+77p1VD5yku4joo9NuunhlkAiX+ eyQEGDWQQEajIe5GgJLxmT1vakch/3ch/2bddwVwbNX/1rwnBdYcTPJmwXWU/A== 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=1723038261; 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=rcFa05g/quTSmsQac8GoTI+wAFqajW5Wo9WxRGL/oxQ=; b=cDbkTPQv49bNZ3/BxfBzODWVRPzEIyUpDQZJFbXgbFH6pbXfPxDxaXpfOipmp6YVtQYhyV kFh3MqjmsoyoOYBYd6+EoEpNAwLGy9dZPH+FipGxmvMVvxdRh3AnQa/+MrnJfzlgufYaBB hYJGsiX7Ko4NcfzgMI6GEk14rcEquyy8L7SVRccmUDctXxpxCR9v9f6SZcgnUv6d2euNO+ tRO8ICQMYinM83fsVTnAch5n2E4HD7VRghpmim3AW6j5I04EAYba9kcDZxwoqf3Mk6GatN SNSASLmZ2THxag2tfArd86BtFj1WCZd8fnHpDtoyYh71Y6rDl9KOJIBgbevqgg== 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 4WfBH939L8zsjP; Wed, 7 Aug 2024 13:44:21 +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 477DiLtl033353; Wed, 7 Aug 2024 13:44:21 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 477DiLSL033350; Wed, 7 Aug 2024 13:44:21 GMT (envelope-from git) Date: Wed, 7 Aug 2024 13:44:21 GMT Message-Id: <202408071344.477DiLSL033350@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Mark Johnston Subject: git: 8533e927afc1 - releng/14.1 - nfscl: Scan readdir reply filenames for invalid characters List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: markj X-Git-Repository: src X-Git-Refname: refs/heads/releng/14.1 X-Git-Reftype: branch X-Git-Commit: 8533e927afc1847fb8ff6b650a69999ec6612df3 Auto-Submitted: auto-generated The branch releng/14.1 has been updated by markj: URL: https://cgit.FreeBSD.org/src/commit/?id=8533e927afc1847fb8ff6b650a69999ec6612df3 commit 8533e927afc1847fb8ff6b650a69999ec6612df3 Author: Rick Macklem AuthorDate: 2024-07-21 22:56:16 +0000 Commit: Mark Johnston CommitDate: 2024-08-07 13:30:29 +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. Approved by: so Security: FreeBSD-SA-24:07.nfsclient Security: CVE-2024-6759 Reported by: Apple Security Engineering and Architecture (SEAR) (cherry picked from commit 026cdaa3b3a92574d9ac3155216e5cc0b0bd4c51) (cherry picked from commit 9328ded386d570c8455b9021e047520ef72e0e79) --- 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 8c5532268287..c5c0ee9fa74f 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); @@ -3279,6 +3280,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 @@ -3331,6 +3357,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 @@ -3588,6 +3616,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; @@ -3603,20 +3642,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) @@ -3782,6 +3836,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; @@ -4017,6 +4073,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; @@ -4035,25 +4102,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)