Skip to content

Commit

Permalink
nfscl: Scan readdir reply filenames for invalid characters
Browse files Browse the repository at this point in the history 
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

(cherry picked from commit 026cdaa3b3a92574d9ac3155216e5cc0b0bd4c51)
  • Loading branch information
@brooksdavis
Rick Macklem authored and brooksdavis committed Aug 7, 2024
1 parent 4e51b18 commit b2f6def
Showing 1 changed file with 108 additions and 23 deletions.
131 changes: 108 additions & 23 deletions sys/fs/nfsclient/nfs_clrpcops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,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);
Expand Down Expand Up @@ -3289,6 +3290,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
Expand Down Expand Up @@ -3341,6 +3367,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
Expand Down Expand Up @@ -3593,6 +3621,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 = (__cheri_fromcap struct dirent *)uiop->uio_iov->iov_base;
dp->d_pad0 = dp->d_pad1 = 0;
dp->d_off = 0;
Expand All @@ -3606,18 +3645,34 @@ nfsrpc_readdir(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep,
uiop->uio_resid -= DIRHDSIZ;
uiop->uio_offset += DIRHDSIZ;
IOVEC_ADVANCE(uiop->uio_iov, DIRHDSIZ);
cp = (__cheri_fromcap char *)
uiop->uio_iov->iov_base;
error = nfsm_mbufuio(nd, uiop, len);
if (error)
goto nfsmout;
cp = (__cheri_fromcap void *)uiop->uio_iov->iov_base;
tlen -= len;
NFSBZERO(cp, tlen);
cp += tlen; /* points to cookie storage */
tl2 = (u_int32_t *)cp;
IOVEC_ADVANCE(uiop->uio_iov,
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 = (__cheri_fromcap char *)
uiop->uio_iov->iov_base;
tlen -= len;
NFSBZERO(cp, tlen);
cp += tlen; /* points to cookie store */
tl2 = (u_int32_t *)cp;
IOVEC_ADVANCE(uiop->uio_iov,
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)
Expand Down Expand Up @@ -3779,6 +3834,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;
Expand Down Expand Up @@ -4008,6 +4065,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 = (__cheri_fromcap struct dirent *)uiop->uio_iov->iov_base;
dp->d_pad0 = dp->d_pad1 = 0;
dp->d_off = 0;
Expand All @@ -4024,23 +4092,40 @@ nfsrpc_readdirplus(vnode_t vp, struct uio *uiop, nfsuint64 *cookiep,
cnp->cn_nameptr = (__cheri_fromcap void *)uiop->uio_iov->iov_base;
cnp->cn_namelen = len;
NFSCNHASHZERO(cnp);
cp = (__cheri_fromcap char *)
uiop->uio_iov->iov_base;
error = nfsm_mbufuio(nd, uiop, len);
if (error)
goto nfsmout;
cp = (__cheri_fromcap void *)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;
IOVEC_ADVANCE(uiop->uio_iov,
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 = (__cheri_fromcap void *)
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;
IOVEC_ADVANCE(uiop->uio_iov,
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)
Expand Down

0 comments on commit b2f6def

Please sign in to comment.