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
This commit is contained in:
Jessica Clarke 2021-10-24 19:49:21 +01:00
parent f350bc1dd3
commit 34fb1c133c
2 changed files with 26 additions and 14 deletions

View File

@ -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) <

View File

@ -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) <