Attachment 168071 Details for Bug 204643 – msdosfs: Prevent buffer overflow when expanding win95 names

[patch] msdosfs: Prevent buffer overflow when expanding win95 names

204643.patch (text/plain), 4.09 KB, created by Kristof Provost on 2016-03-13 04:20:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kristof Provost

Created: 2016-03-13 04:20:43 UTC

Size: 4.09 KB

patch

obsolete

>commit e24ba8b6c403f8c706a9aa0dd64050c2449a9fca
>Author: Kristof Provost <kristof@codepro.be>
>Date:   Sun Mar 13 06:14:51 2016 +0100
>
>    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
>
>diff --git a/sys/fs/msdosfs/direntry.h b/sys/fs/msdosfs/direntry.h
>index 86b6fbb..facec30 100644
>--- a/sys/fs/msdosfs/direntry.h
>+++ b/sys/fs/msdosfs/direntry.h
>@@ -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,
>diff --git a/sys/fs/msdosfs/msdosfs_conv.c b/sys/fs/msdosfs/msdosfs_conv.c
>index fc9b4d4..e584d69 100644
>--- a/sys/fs/msdosfs/msdosfs_conv.c
>+++ b/sys/fs/msdosfs/msdosfs_conv.c
>@@ -634,6 +634,7 @@ win2unixfn(struct mbnambuf *nbp, struct winentry *wep, int chksum,
> 	u_int8_t *np, name[WIN_CHARS * 3 + 1];
> 	u_int16_t code;
> 	int i;
>+	int ret;
> 
> 	if ((wep->weCnt&WIN_CNT) > howmany(WIN_MAXLEN, WIN_CHARS)
> 	    || !(wep->weCnt&WIN_CNT))
>@@ -658,7 +659,9 @@ win2unixfn(struct mbnambuf *nbp, struct winentry *wep, int chksum,
> 		switch (code) {
> 		case 0:
> 			*np = '\0';
>-			mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+			ret = mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+			if (ret < 0)
>+				return (-1);
> 			return chksum;
> 		case '/':
> 			*np = '\0';
>@@ -676,7 +679,9 @@ win2unixfn(struct mbnambuf *nbp, struct winentry *wep, int chksum,
> 		switch (code) {
> 		case 0:
> 			*np = '\0';
>-			mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+			ret = mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+			if (ret < 0)
>+				return (-1);
> 			return chksum;
> 		case '/':
> 			*np = '\0';
>@@ -694,7 +699,9 @@ win2unixfn(struct mbnambuf *nbp, struct winentry *wep, int chksum,
> 		switch (code) {
> 		case 0:
> 			*np = '\0';
>-			mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+			ret = mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+			if (ret < 0)
>+				return (-1);
> 			return chksum;
> 		case '/':
> 			*np = '\0';
>@@ -708,7 +715,9 @@ win2unixfn(struct mbnambuf *nbp, struct winentry *wep, int chksum,
> 		cp += 2;
> 	}
> 	*np = '\0';
>-	mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+	ret = mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
>+	if (ret < 0)
>+		return (-1);
> 	return chksum;
> }
> 
>@@ -1005,7 +1014,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;
>@@ -1016,7 +1025,7 @@ mbnambuf_write(struct mbnambuf *nbp, char *name, int id)
> 		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. */
>@@ -1027,17 +1036,24 @@ mbnambuf_write(struct mbnambuf *nbp, char *name, int id)
> #ifdef MSDOSFS_DEBUG
> 		printf("msdosfs: file name length %zu too large\n", newlen);
> #endif
>-		return;
>+		return (-ENOMEM);
> 	}
> 
> 	/* Shift suffix upwards by the amount length exceeds WIN_CHARS. */
> 	if (count > WIN_CHARS && nbp->nb_len != 0)
>+	{
>+		if (((slot - nbp->nb_buf) + count + nbp->nb_len) > sizeof(nbp->nb_buf))
>+			return (-ENOMEM);
>+
> 		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);
> }
> 
> /*

Actions: View | Diff

Attachments on bug 204643: 163261 | 168071