git: 34fb1c133c5b - main - Fix intra-object buffer overread for labeled msdosfs volumes

From: Jessica Clarke <jrtc27_at_FreeBSD.org>
Date: Wed, 27 Oct 2021 17:38:47 UTC
The branch main has been updated by jrtc27:

URL: https://cgit.FreeBSD.org/src/commit/?id=34fb1c133c5b8616f14f1d740d99747b427f5571

commit 34fb1c133c5b8616f14f1d740d99747b427f5571
Author:     Jessica Clarke <jrtc27@FreeBSD.org>
AuthorDate: 2021-10-24 18:49:21 +0000
Commit:     Jessica Clarke <jrtc27@FreeBSD.org>
CommitDate: 2021-10-27 17:38:37 +0000

    Fix intra-object buffer overread for labeled msdosfs volumes
    
    Volume labels, like directory entries, are padded with spaces and so
    have no NUL terminator. Whilst the MIN for the dsize argument to strlcpy
    ensures that the copy does not overflow the destination, strlcpy is
    defined to return the number of characters in the source string,
    regardless of the provided dsize, and so keeps reading until it finds a
    NUL, which likely exists somewhere within the following fields, but On
    CHERI with the subobject bounds enabled in the compiler this buffer
    overread will be detected and trap with a bounds violation.
    
    Found by:       CHERI
    Reviewed by:    imp
    Differential Revision:  https://reviews.freebsd.org/D32579
---
 sys/geom/label/g_label_msdosfs.c | 20 +++++++++++++-------
 usr.sbin/fstyp/msdosfs.c         | 20 +++++++++++++-------
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/sys/geom/label/g_label_msdosfs.c b/sys/geom/label/g_label_msdosfs.c
index d6ccb8b334ee..2ba35ff80f51 100644
--- a/sys/geom/label/g_label_msdosfs.c
+++ b/sys/geom/label/g_label_msdosfs.c
@@ -50,6 +50,7 @@ g_label_msdosfs_taste(struct g_consumer *cp, char *label, size_t size)
 	FAT32_BSBPB *pfat32_bsbpb;
 	FAT_DES *pfat_entry;
 	uint8_t *sector0, *sector;
+	size_t copysize;
 
 	g_topology_assert_not();
 	pp = cp->provider;
@@ -111,8 +112,9 @@ g_label_msdosfs_taste(struct g_consumer *cp, char *label, size_t size)
 			    pp->name);
 			goto error;
 		}
-		strlcpy(label, pfat_bsbpb->BS_VolLab,
-		    MIN(size, sizeof(pfat_bsbpb->BS_VolLab) + 1));
+		copysize = MIN(size - 1, sizeof(pfat_bsbpb->BS_VolLab));
+		memcpy(label, pfat_bsbpb->BS_VolLab, copysize);
+		label[copysize] = '\0';
 	} else if (UINT32BYTES(pfat32_bsbpb->BPB_FATSz32) != 0) {
 		uint32_t fat_FirstDataSector, fat_BytesPerSector, offset;
 
@@ -133,8 +135,10 @@ g_label_msdosfs_taste(struct g_consumer *cp, char *label, size_t size)
 		 */
 		if (strncmp(pfat32_bsbpb->BS_VolLab, LABEL_NO_NAME,
 		    sizeof(pfat32_bsbpb->BS_VolLab)) != 0) {
-			strlcpy(label, pfat32_bsbpb->BS_VolLab,
-			    MIN(size, sizeof(pfat32_bsbpb->BS_VolLab) + 1));
+			copysize = MIN(size - 1,
+			    sizeof(pfat32_bsbpb->BS_VolLab) + 1);
+			memcpy(label, pfat32_bsbpb->BS_VolLab, copysize);
+			label[copysize] = '\0';
 			goto endofchecks;
 		}
 
@@ -184,9 +188,11 @@ g_label_msdosfs_taste(struct g_consumer *cp, char *label, size_t size)
 				 */
 				if (pfat_entry->DIR_Attr &
 				    FAT_DES_ATTR_VOLUME_ID) {
-					strlcpy(label, pfat_entry->DIR_Name,
-					    MIN(size,
-					    sizeof(pfat_entry->DIR_Name) + 1));
+					copysize = MIN(size - 1,
+					    sizeof(pfat_entry->DIR_Name));
+					memcpy(label, pfat_entry->DIR_Name,
+					    copysize);
+					label[copysize] = '\0';
 					goto endofchecks;
 				}
 			} while((uint8_t *)(++pfat_entry) <
diff --git a/usr.sbin/fstyp/msdosfs.c b/usr.sbin/fstyp/msdosfs.c
index 3d86802f6a2e..ce745869edba 100644
--- a/usr.sbin/fstyp/msdosfs.c
+++ b/usr.sbin/fstyp/msdosfs.c
@@ -48,6 +48,7 @@ fstyp_msdosfs(FILE *fp, char *label, size_t size)
 	FAT32_BSBPB *pfat32_bsbpb;
 	FAT_DES *pfat_entry;
 	uint8_t *sector0, *sector;
+	size_t copysize;
 
 	sector0 = NULL;
 	sector = NULL;
@@ -83,8 +84,9 @@ fstyp_msdosfs(FILE *fp, char *label, size_t size)
 		    sizeof(pfat_bsbpb->BS_VolLab)) == 0) {
 			goto endofchecks;
 		}
-		strlcpy(label, pfat_bsbpb->BS_VolLab,
-		    MIN(size, sizeof(pfat_bsbpb->BS_VolLab) + 1));
+		copysize = MIN(size - 1, sizeof(pfat_bsbpb->BS_VolLab));
+		memcpy(label, pfat_bsbpb->BS_VolLab, copysize);
+		label[copysize] = '\0';
 	} else if (UINT32BYTES(pfat32_bsbpb->BPB_FATSz32) != 0) {
 		uint32_t fat_FirstDataSector, fat_BytesPerSector, offset;
 
@@ -101,8 +103,10 @@ fstyp_msdosfs(FILE *fp, char *label, size_t size)
 		 */
 		if (strncmp(pfat32_bsbpb->BS_VolLab, LABEL_NO_NAME,
 		    sizeof(pfat32_bsbpb->BS_VolLab)) != 0) {
-			strlcpy(label, pfat32_bsbpb->BS_VolLab,
-			    MIN(size, sizeof(pfat32_bsbpb->BS_VolLab) + 1));
+			copysize = MIN(size - 1,
+			    sizeof(pfat32_bsbpb->BS_VolLab) + 1);
+			memcpy(label, pfat32_bsbpb->BS_VolLab, copysize);
+			label[copysize] = '\0';
 			goto endofchecks;
 		}
 
@@ -146,9 +150,11 @@ fstyp_msdosfs(FILE *fp, char *label, size_t size)
 				 */
 				if (pfat_entry->DIR_Attr &
 				    FAT_DES_ATTR_VOLUME_ID) {
-					strlcpy(label, pfat_entry->DIR_Name,
-					    MIN(size,
-					    sizeof(pfat_entry->DIR_Name) + 1));
+					copysize = MIN(size - 1,
+					    sizeof(pfat_entry->DIR_Name));
+					memcpy(label, pfat_entry->DIR_Name,
+					    copysize);
+					label[copysize] = '\0';
 					goto endofchecks;
 				}
 			} while((uint8_t *)(++pfat_entry) <