From owner-svn-src-head@freebsd.org Fri Oct 30 19:00:43 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 71A08459621; Fri, 30 Oct 2020 19:00:43 +0000 (UTC) (envelope-from cem@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 "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4CNBTl2DJ2z4N13; Fri, 30 Oct 2020 19:00:43 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 2FCF71AE58; Fri, 30 Oct 2020 19:00:43 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 09UJ0hd8040199; Fri, 30 Oct 2020 19:00:43 GMT (envelope-from cem@FreeBSD.org) Received: (from cem@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 09UJ0glY040197; Fri, 30 Oct 2020 19:00:42 GMT (envelope-from cem@FreeBSD.org) Message-Id: <202010301900.09UJ0glY040197@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: cem set sender to cem@FreeBSD.org using -f From: Conrad Meyer Date: Fri, 30 Oct 2020 19:00:42 +0000 (UTC) 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 X-SVN-Group: head X-SVN-Commit-Author: cem X-SVN-Commit-Paths: in head/sys/ufs: ffs ufs X-SVN-Commit-Revision: 367181 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 30 Oct 2020 19:00:43 -0000 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 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) \