Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 29 Apr 2016 20:19:41 +0000 (UTC)
From:      Kristof Provost <kp@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r298799 - stable/10/sys/fs/msdosfs
Message-ID:  <201604292019.u3TKJfi3007069@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: kp
Date: Fri Apr 29 20:19:41 2016
New Revision: 298799
URL: https://svnweb.freebsd.org/changeset/base/298799

Log:
  MFC r298664
  
  msdosfs: Prevent buffer overflow when expanding win95 names
  
  In win2unixfn() we expand Windows 95 style long names. In some cases that
  requires moving the data in the nbp->nb_buf buffer backwards to make room. That
  code failed to check for overflows, leading to a stack overflow in win2unixfn().
  
  We now check for this event, and mark the entire conversion as failed in that
  case. This means we present the 8 character, dos style, name instead.
  
  PR: 204643
  Differential Revision:      https://reviews.freebsd.org/D6015

Modified:
  stable/10/sys/fs/msdosfs/direntry.h
  stable/10/sys/fs/msdosfs/msdosfs_conv.c
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/fs/msdosfs/direntry.h
==============================================================================
--- stable/10/sys/fs/msdosfs/direntry.h	Fri Apr 29 20:13:35 2016	(r298798)
+++ stable/10/sys/fs/msdosfs/direntry.h	Fri Apr 29 20:19:41 2016	(r298799)
@@ -145,7 +145,7 @@ struct msdosfsmount;
 
 char	*mbnambuf_flush(struct mbnambuf *nbp, struct dirent *dp);
 void	mbnambuf_init(struct mbnambuf *nbp);
-void	mbnambuf_write(struct mbnambuf *nbp, char *name, int id);
+int	mbnambuf_write(struct mbnambuf *nbp, char *name, int id);
 int	dos2unixfn(u_char dn[11], u_char *un, int lower,
 	    struct msdosfsmount *pmp);
 int	unix2dosfn(const u_char *un, u_char dn[12], size_t unlen, u_int gen,

Modified: stable/10/sys/fs/msdosfs/msdosfs_conv.c
==============================================================================
--- stable/10/sys/fs/msdosfs/msdosfs_conv.c	Fri Apr 29 20:13:35 2016	(r298798)
+++ stable/10/sys/fs/msdosfs/msdosfs_conv.c	Fri Apr 29 20:19:41 2016	(r298799)
@@ -678,7 +678,9 @@ win2unixfn(nbp, wep, chksum, pmp)
 		switch (code) {
 		case 0:
 			*np = '\0';
-			mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
+			if (mbnambuf_write(nbp, name,
+			    (wep->weCnt & WIN_CNT) - 1) != 0)
+				return -1;
 			return chksum;
 		case '/':
 			*np = '\0';
@@ -696,7 +698,9 @@ win2unixfn(nbp, wep, chksum, pmp)
 		switch (code) {
 		case 0:
 			*np = '\0';
-			mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
+			if (mbnambuf_write(nbp, name,
+			    (wep->weCnt & WIN_CNT) - 1) != 0)
+				return -1;
 			return chksum;
 		case '/':
 			*np = '\0';
@@ -714,7 +718,9 @@ win2unixfn(nbp, wep, chksum, pmp)
 		switch (code) {
 		case 0:
 			*np = '\0';
-			mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
+			if (mbnambuf_write(nbp, name,
+			    (wep->weCnt & WIN_CNT) - 1) != 0)
+				return -1;
 			return chksum;
 		case '/':
 			*np = '\0';
@@ -728,7 +734,8 @@ win2unixfn(nbp, wep, chksum, pmp)
 		cp += 2;
 	}
 	*np = '\0';
-	mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
+	if (mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1) != 0)
+		return -1;
 	return chksum;
 }
 
@@ -1030,7 +1037,7 @@ mbnambuf_init(struct mbnambuf *nbp)
  * This only penalizes portions of substrings that contain more than
  * WIN_CHARS bytes when they are first encountered.
  */
-void
+int
 mbnambuf_write(struct mbnambuf *nbp, char *name, int id)
 {
 	char *slot;
@@ -1041,7 +1048,7 @@ mbnambuf_write(struct mbnambuf *nbp, cha
 		printf("msdosfs: non-decreasing id: id %d, last id %d\n",
 		    id, nbp->nb_last_id);
 #endif
-		return;
+		return (EINVAL);
 	}
 
 	/* Will store this substring in a WIN_CHARS-aligned slot. */
@@ -1052,17 +1059,24 @@ mbnambuf_write(struct mbnambuf *nbp, cha
 #ifdef MSDOSFS_DEBUG
 		printf("msdosfs: file name length %zu too large\n", newlen);
 #endif
-		return;
+		return (ENAMETOOLONG);
 	}
 
 	/* Shift suffix upwards by the amount length exceeds WIN_CHARS. */
-	if (count > WIN_CHARS && nbp->nb_len != 0)
+	if (count > WIN_CHARS && nbp->nb_len != 0) {
+		if ((id * WIN_CHARS + count + nbp->nb_len) >
+		    sizeof(nbp->nb_buf))
+			return (ENAMETOOLONG);
+
 		bcopy(slot + WIN_CHARS, slot + count, nbp->nb_len);
+	}
 
 	/* Copy in the substring to its slot and update length so far. */
 	bcopy(name, slot, count);
 	nbp->nb_len = newlen;
 	nbp->nb_last_id = id;
+
+	return (0);
 }
 
 /*



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