Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 30 Oct 2020 19:00:42 +0000 (UTC)
From:      Conrad Meyer <cem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r367181 - in head/sys/ufs: ffs ufs
Message-ID:  <202010301900.09UJ0glY040197@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: cem
Date: Fri Oct 30 19:00:42 2020
New Revision: 367181
URL: https://svnweb.freebsd.org/changeset/base/367181

Log:
  UFS2: Fix DoS due to corrupted extattrfile
  
  Prior versions of FreeBSD (11.x) may have produced a corrupt extattr file.
  (Specifically, r312416 accidentally fixed this defect by removing a strcpy.)
  CURRENT FreeBSD supports disk images from those prior versions of FreeBSD.
  Validate the internal structure as soon as we read it in from disk, to
  prevent these extattr files from causing invariants violations and DoS.
  
  Attempting to access the extattr portion of these files results in
  EINTEGRITY.  At this time, the only way to repair files damaged in this way
  is to copy the contents to another file and move it over the original.
  
  PR:		244089
  Reported by:	Andrea Venturoli <ml AT netfence.it>
  Reviewed by:	kib
  Discussed with:	mckusick (earlier draft)
  Security:	no
  Differential Revision:	https://reviews.freebsd.org/D27010

Modified:
  head/sys/ufs/ffs/ffs_vnops.c
  head/sys/ufs/ufs/extattr.h

Modified: head/sys/ufs/ffs/ffs_vnops.c
==============================================================================
--- head/sys/ufs/ffs/ffs_vnops.c	Fri Oct 30 18:55:08 2020	(r367180)
+++ head/sys/ufs/ffs/ffs_vnops.c	Fri Oct 30 19:00:42 2020	(r367181)
@@ -1200,9 +1200,8 @@ ffs_findextattr(u_char *ptr, u_int length, int nspace,
 	eap = (struct extattr *)ptr;
 	eaend = (struct extattr *)(ptr + length);
 	for (; eap < eaend; eap = EXTATTR_NEXT(eap)) {
-		/* make sure this entry is complete */
-		if (EXTATTR_NEXT(eap) > eaend)
-			break;
+		KASSERT(EXTATTR_NEXT(eap) <= eaend,
+		    ("extattr next %p beyond %p", EXTATTR_NEXT(eap), eaend));
 		if (eap->ea_namespace != nspace || eap->ea_namelength != nlen
 		    || memcmp(eap->ea_name, name, nlen) != 0)
 			continue;
@@ -1216,8 +1215,9 @@ ffs_findextattr(u_char *ptr, u_int length, int nspace,
 }
 
 static int
-ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td, int extra)
+ffs_rdextattr(u_char **p, struct vnode *vp, struct thread *td)
 {
+	const struct extattr *eap, *eaend, *eapnext;
 	struct inode *ip;
 	struct ufs2_dinode *dp;
 	struct fs *fs;
@@ -1231,10 +1231,10 @@ ffs_rdextattr(u_char **p, struct vnode *vp, struct thr
 	fs = ITOFS(ip);
 	dp = ip->i_din2;
 	easize = dp->di_extsize;
-	if ((uoff_t)easize + extra > UFS_NXADDR * fs->fs_bsize)
+	if ((uoff_t)easize > UFS_NXADDR * fs->fs_bsize)
 		return (EFBIG);
 
-	eae = malloc(easize + extra, M_TEMP, M_WAITOK);
+	eae = malloc(easize, M_TEMP, M_WAITOK);
 
 	liovec.iov_base = eae;
 	liovec.iov_len = easize;
@@ -1249,8 +1249,18 @@ ffs_rdextattr(u_char **p, struct vnode *vp, struct thr
 	error = ffs_extread(vp, &luio, IO_EXT | IO_SYNC);
 	if (error) {
 		free(eae, M_TEMP);
-		return(error);
+		return (error);
 	}
+	/* Validate disk xattrfile contents. */
+	for (eap = (void *)eae, eaend = (void *)(eae + easize); eap < eaend;
+	    eap = eapnext) {
+		eapnext = EXTATTR_NEXT(eap);
+		/* Bogusly short entry or bogusly long entry. */
+		if (eap->ea_length < sizeof(*eap) || eapnext > eaend) {
+			free(eae, M_TEMP);
+			return (EINTEGRITY);
+		}
+	}
 	*p = eae;
 	return (0);
 }
@@ -1300,7 +1310,7 @@ ffs_open_ea(struct vnode *vp, struct ucred *cred, stru
 		return (0);
 	}
 	dp = ip->i_din2;
-	error = ffs_rdextattr(&ip->i_ea_area, vp, td, 0);
+	error = ffs_rdextattr(&ip->i_ea_area, vp, td);
 	if (error) {
 		ffs_unlock_ea(vp);
 		return (error);
@@ -1606,9 +1616,8 @@ vop_listextattr {
 	eap = (struct extattr *)ip->i_ea_area;
 	eaend = (struct extattr *)(ip->i_ea_area + ip->i_ea_len);
 	for (; error == 0 && eap < eaend; eap = EXTATTR_NEXT(eap)) {
-		/* make sure this entry is complete */
-		if (EXTATTR_NEXT(eap) > eaend)
-			break;
+		KASSERT(EXTATTR_NEXT(eap) <= eaend,
+		    ("extattr next %p beyond %p", EXTATTR_NEXT(eap), eaend));
 		if (eap->ea_namespace != ap->a_attrnamespace)
 			continue;
 

Modified: head/sys/ufs/ufs/extattr.h
==============================================================================
--- head/sys/ufs/ufs/extattr.h	Fri Oct 30 18:55:08 2020	(r367180)
+++ head/sys/ufs/ufs/extattr.h	Fri Oct 30 19:00:42 2020	(r367181)
@@ -95,7 +95,7 @@ struct extattr {
  *	content referenced by eap.
  */
 #define	EXTATTR_NEXT(eap) \
-	((struct extattr *)(((u_char *)(eap)) + (eap)->ea_length))
+	((struct extattr *)(__DECONST(char *, (eap)) + (eap)->ea_length))
 #define	EXTATTR_CONTENT(eap) \
 	(void *)(((u_char *)(eap)) + EXTATTR_BASE_LENGTH(eap))
 #define	EXTATTR_CONTENT_SIZE(eap) \

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202010301900.09UJ0glY040197>