diff options
author | Jessica Clarke <jrtc27@FreeBSD.org> | 2021-10-24 18:49:21 +0000 |
---|---|---|
committer | Jessica Clarke <jrtc27@FreeBSD.org> | 2021-10-27 17:38:37 +0000 |
commit | 34fb1c133c5b8616f14f1d740d99747b427f5571 (patch) | |
tree | b235179bf03dc8f54d3f53be6d6370e314a9c75e /usr.sbin | |
parent | f350bc1dd368a3024ba9ef2a9e8431fc1edd8094 (diff) | |
download | src-34fb1c133c5b8616f14f1d740d99747b427f5571.tar.gz src-34fb1c133c5b8616f14f1d740d99747b427f5571.zip |
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
Diffstat (limited to 'usr.sbin')
-rw-r--r-- | usr.sbin/fstyp/msdosfs.c | 20 |
1 files changed, 13 insertions, 7 deletions
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) < |