HBSD MFC: This update eliminates a kernel stack disclosure bug in UFS…

…/FFS

directory entries that is caused by uninitialized directory entry
padding written to the disk. It can be viewed by any user with read
access to that directory. Up to 3 bytes of kernel stack are disclosed
per file entry, depending on the the amount of padding the kernel
needs to pad out the entry to a 32 bit boundry. The offset in the
kernel stack that is disclosed is a function of the filename size.
Furthermore, if the user can create files in a directory, this 3
byte window can be expanded 3 bytes at a time to a 254 byte window
with 75% of the data in that window exposed. The additional exposure
is done by removing the entry, creating a new entry with a 4-byte
longer name, extracting 3 more bytes by reading the directory, and
repeating until a 252 byte name is created.

This exploit works in part because the area of the kernel stack
that is being disclosed is in an area that typically doesn't change
that often (perhaps a few times a second on a lightly loaded system),
and these file creates and unlinks themselves don't overwrite the
area of kernel stack being disclosed.

It appears that this bug originated with the creation of the Fast
File System in 4.1b-BSD (Circa 1982, more than 36 years ago!), and
is likely present in every Unix or Unix-like system that uses
UFS/FFS. Amazingly, nobody noticed until now.

This update also adds the -z flag to fsck_ffs to have it scrub
the leaked information in the name padding of existing directories.
It only needs to be run once on each UFS/FFS filesystem after a
patched kernel is installed and running.

Submitted by: David G. Lawrence <dg@dglawrence.com>
Reviewed by:  kib
MFC after:    1 week

(cherry picked from commit c4824e67f54b25d500b4371e14f62df3882f2f5e)
Signed-off-by: Shawn Webb <shawn.webb@hardenedbsd.org>

sbin/fsck_ffs/dir.c

@@ -147,14 +147,23 @@ fsck_readdir(struct inodesc *idesc)
 	struct direct *dp, *ndp;
 	struct bufarea *bp;
 	long size, blksiz, fix, dploc;
+	int dc;
 
 	blksiz = idesc->id_numfrags * sblock.fs_fsize;
 	bp = getdirblk(idesc->id_blkno, blksiz);
 	if (idesc->id_loc % DIRBLKSIZ == 0 && idesc->id_filesize > 0 &&
 	    idesc->id_loc < blksiz) {
 		dp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc);
-		if (dircheck(idesc, dp))
+		if ((dc = dircheck(idesc, dp)) > 0) {
+			if (dc == 2) {
+				/*
+				 * dircheck() cleared unused directory space.
+				 * Mark the buffer as dirty to write it out.
+				 */
+				dirty(bp);
+			}
 			goto dpok;
+		}
 		if (idesc->id_fix == IGNORE)
 			return (0);
 		fix = dofix(idesc, "DIRECTORY CORRUPTED");
@@ -181,26 +190,38 @@ fsck_readdir(struct inodesc *idesc)
 	if ((idesc->id_loc % DIRBLKSIZ) == 0)
 		return (dp);
 	ndp = (struct direct *)(bp->b_un.b_buf + idesc->id_loc);
-	if (idesc->id_loc < blksiz && idesc->id_filesize > 0 &&
-	    dircheck(idesc, ndp) == 0) {
-		size = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ);
-		idesc->id_loc += size;
-		idesc->id_filesize -= size;
-		if (idesc->id_fix == IGNORE)
-			return (0);
-		fix = dofix(idesc, "DIRECTORY CORRUPTED");
-		bp = getdirblk(idesc->id_blkno, blksiz);
-		dp = (struct direct *)(bp->b_un.b_buf + dploc);
-		dp->d_reclen += size;
-		if (fix)
+	if (idesc->id_loc < blksiz && idesc->id_filesize > 0) {
+		if ((dc = dircheck(idesc, ndp)) == 0) {
+			size = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ);
+			idesc->id_loc += size;
+			idesc->id_filesize -= size;
+			if (idesc->id_fix == IGNORE)
+				return (0);
+			fix = dofix(idesc, "DIRECTORY CORRUPTED");
+			bp = getdirblk(idesc->id_blkno, blksiz);
+			dp = (struct direct *)(bp->b_un.b_buf + dploc);
+			dp->d_reclen += size;
+			if (fix)
+				dirty(bp);
+		} else if (dc == 2) {
+			/*
+			 * dircheck() cleared unused directory space.
+			 * Mark the buffer as dirty to write it out.
+			 */
 			dirty(bp);
+		}
 	}
 	return (dp);
 }
 
 /*
  * Verify that a directory entry is valid.
  * This is a superset of the checks made in the kernel.
+ * Also optionally clears padding and unused directory space.
+ *
+ * Returns 0 if the entry is bad, 1 if the entry is good and no changes
+ * were made, and 2 if the entry is good but modified to clear out padding
+ * and unused space and needs to be written back to disk.
  */
 static int
 dircheck(struct inodesc *idesc, struct direct *dp)
@@ -209,15 +230,39 @@ dircheck(struct inodesc *idesc, struct direct *dp)
 	char *cp;
 	u_char type;
 	u_int8_t namlen;
-	int spaceleft;
+	int spaceleft, modified, unused;
 
+	modified = 0;
 	spaceleft = DIRBLKSIZ - (idesc->id_loc % DIRBLKSIZ);
 	if (dp->d_reclen == 0 ||
 	    dp->d_reclen > spaceleft ||
-	    (dp->d_reclen & 0x3) != 0)
+	    (dp->d_reclen & (DIR_ROUNDUP - 1)) != 0)
 		goto bad;
-	if (dp->d_ino == 0)
-		return (1);
+	if (dp->d_ino == 0) {
+		/*
+		 * Special case of an unused directory entry. Normally
+		 * the kernel would coalesce unused space with the previous
+		 * entry by extending its d_reclen, but there are situations
+		 * (e.g. fsck) where that doesn't occur.
+		 * If we're clearing out directory cruft (-z flag), then make
+		 * sure this entry gets fully cleared as well.
+		 */
+		if (zflag && fswritefd >= 0) {
+			if (dp->d_type != 0) {
+				dp->d_type = 0;
+				modified = 1;
+			}
+			if (dp->d_namlen != 0) {
+				dp->d_namlen = 0;
+				modified = 1;
+			}
+			if (dp->d_name[0] != '\0') {
+				dp->d_name[0] = '\0';
+				modified = 1;
+			}
+		}
+		goto good;
+	}
 	size = DIRSIZ(0, dp);
 	namlen = dp->d_namlen;
 	type = dp->d_type;
@@ -231,7 +276,37 @@ dircheck(struct inodesc *idesc, struct direct *dp)
 			goto bad;
 	if (*cp != '\0')
 		goto bad;
+
+good:
+	if (zflag && fswritefd >= 0) {
+		/*
+		 * Clear unused directory entry space, including the d_name
+		 * padding.
+		 */
+		/* First figure the number of pad bytes. */
+		unused = roundup2(namlen + 1, DIR_ROUNDUP) - (namlen + 1);
+
+		/* Add in the free space to the end of the record. */
+		unused += dp->d_reclen - DIRSIZ(0, dp);
+
+		/*
+		 * Now clear out the unused space, keeping track if we actually
+		 * changed anything.
+		 */
+		for (cp = &dp->d_name[namlen + 1]; unused > 0; unused--, cp++) {
+			if (*cp != '\0') {
+				*cp = '\0';
+				modified = 1;
+			}
+		}
+
+		if (modified) {
+			return 2;
+		}
+	}
+
 	return (1);
+
 bad:
 	if (debug)
 		printf("Bad dir: ino %d reclen %d namlen %d type %d name %s\n",

sbin/fsck_ffs/fsck.h

@@ -314,6 +314,7 @@ extern off_t	bflag;			/* location of alternate super block */
 extern int	debug;			/* output debugging info */
 extern int	Eflag;			/* delete empty data blocks */
 extern int	Zflag;			/* zero empty data blocks */
+extern int	zflag;			/* zero unused directory space */
 extern int	inoopt;			/* trim out unused inodes */
 extern char	ckclean;		/* only do work if not cleanly unmounted */
 extern int	cvtlevel;		/* convert to newer file system format */

sbin/fsck_ffs/fsck_ffs.8

@@ -29,7 +29,7 @@
 .\"	@(#)fsck.8	8.4 (Berkeley) 5/9/95
 .\" $FreeBSD$
 .\"
-.Dd January 13, 2018
+.Dd May 3, 2019
 .Dt FSCK_FFS 8
 .Os
 .Sh NAME
@@ -38,7 +38,7 @@
 .Nd file system consistency check and interactive repair
 .Sh SYNOPSIS
 .Nm
-.Op Fl BCdEFfnpRrSyZ
+.Op Fl BCdEFfnpRrSyZz
 .Op Fl b Ar block
 .Op Fl c Ar level
 .Op Fl m Ar mode
@@ -301,6 +301,9 @@ If both
 and
 .Fl Z
 are specified, blocks are first zeroed and then erased.
+.It Fl z
+Clear unused directory space.
+The cleared space includes deleted file names and name padding.
 .El
 .Pp
 Inconsistencies checked are as follows:

sbin/fsck_ffs/globs.c

@@ -85,6 +85,7 @@ off_t	bflag;			/* location of alternate super block */
 int	debug;			/* output debugging info */
 int	Eflag;			/* delete empty data blocks */
 int	Zflag;			/* zero empty data blocks */
+int	zflag;			/* zero unused directory space */
 int	inoopt;			/* trim out unused inodes */
 char	ckclean;		/* only do work if not cleanly unmounted */
 int	cvtlevel;		/* convert to newer file system format */

sbin/fsck_ffs/main.c

@@ -89,7 +89,7 @@ main(int argc, char *argv[])
 	sync();
 	skipclean = 1;
 	inoopt = 0;
-	while ((ch = getopt(argc, argv, "b:Bc:CdEfFm:npRrSyZ")) != -1) {
+	while ((ch = getopt(argc, argv, "b:Bc:CdEfFm:npRrSyZz")) != -1) {
 		switch (ch) {
 		case 'b':
 			skipclean = 0;
@@ -166,6 +166,10 @@ main(int argc, char *argv[])
 			Zflag++;
 			break;
 
+		case 'z':
+			zflag++;
+			break;
+
 		default:
 			usage();
 		}

sys/ufs/ufs/ufs_lookup.c

@@ -825,14 +825,21 @@ ufs_makedirentry(ip, cnp, newdirp)
 	struct componentname *cnp;
 	struct direct *newdirp;
 {
+	u_int namelen;
 
-#ifdef INVARIANTS
-	if ((cnp->cn_flags & SAVENAME) == 0)
-		panic("ufs_makedirentry: missing name");
-#endif
+	namelen = (unsigned)cnp->cn_namelen;
+	KASSERT((cnp->cn_flags & SAVENAME) != 0,
+		("ufs_makedirentry: missing name"));
+	KASSERT(namelen <= UFS_MAXNAMLEN,
+		("ufs_makedirentry: name too long"));
 	newdirp->d_ino = ip->i_number;
-	newdirp->d_namlen = cnp->cn_namelen;
-	bcopy(cnp->cn_nameptr, newdirp->d_name, (unsigned)cnp->cn_namelen + 1);
+	newdirp->d_namlen = namelen;
+
+	/* Zero out after-name padding */
+	*(u_int32_t *)(&newdirp->d_name[namelen & ~(DIR_ROUNDUP - 1)]) = 0;
+
+	bcopy(cnp->cn_nameptr, newdirp->d_name, namelen);
+
 	if (ITOV(ip)->v_mount->mnt_maxsymlinklen > 0)
 		newdirp->d_type = IFTODT(ip->i_mode);
 	else {