git: 34fb1c133c5b - main - Fix intra-object buffer overread for labeled msdosfs volumes
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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) <