CVE-2016-4804: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: The read_boot function in boot.c in dosfstools before 4.0 allows attackers to cause a denial of service (crash) via a crafted filesystem, which triggers a heap-based buffer overflow in the (1) read_fat function or an out-of-bounds heap read in (2) get_fat function.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
CVSS v3 base score: 6.2
Published: 2016-06-03T14:59Z
Repository: https://github.com/dosfstools/dosfstools
Language: C
Files changed: src/boot.c, src/fsck.fat.h
Commit date: 2016-05-04 04:28:05 +0200
Commit message: read_boot(): Handle excessive FAT size specifications

The variable used for storing the FAT size (in bytes) was an unsigned
int. Since the size in sectors read from the BPB was not sufficiently
checked, this could end up being zero after multiplying it with the
sector size while some offsets still stayed excessive. Ultimately it
would cause segfaults when accessing FAT entries for which no memory
was allocated.

Make it more robust by changing the types used to store FAT size to
off_t and abort if there is no room for data clusters. Additionally
check that FAT size is not specified as zero.

Fixes #25 and fixes #26.

Reported-by: Hanno Böck
Signed-off-by: Andreas Bombe <aeb@debian.org>

Diff: diff --git a/src/boot.c b/src/boot.c
index 98b5ff9..491ecd9 100644
--- a/src/boot.c
+++ b/src/boot.c
@@ -101,12 +101,12 @@ static void dump_boot(DOS_FS * fs, struct boot_sector *b, unsigned lss)
 	   le16toh(b->reserved) == 1 ? "" : "s");
     printf("First FAT starts at byte %llu (sector %llu)\n",
 	   (unsigned long long)fs->fat_start,
 	   (unsigned long long)fs->fat_start / lss);
     printf("%10d FATs, %d bit entries\n", b->fats, fs->fat_bits);
-    printf("%10d bytes per FAT (= %u sectors)\n", fs->fat_size,
-	   fs->fat_size / lss);
+    printf("%10lld bytes per FAT (= %llu sectors)\n", (long long)fs->fat_size,
+	   (long long)fs->fat_size / lss);
     if (!fs->root_cluster) {
 	printf("Root directory starts at byte %llu (sector %llu)\n",
 	       (unsigned long long)fs->root_start,
 	       (unsigned long long)fs->root_start / lss);
 	printf("%10d root directory entries\n", fs->root_entries);
@@ -327,11 +327,11 @@ static void check_fat_state_bit(DOS_FS * fs, void *b)
 void read_boot(DOS_FS * fs)
 {
     struct boot_sector b;
     unsigned total_sectors;
     unsigned short logical_sector_size, sectors;
-    unsigned fat_length;
+    off_t fat_length;
     unsigned total_fat_entries;
     off_t data_size;
 
     fs_read(0, sizeof(b), &b);
     logical_sector_size = GET_UNALIGNED_W(b.sector_size);
@@ -356,20 +356,28 @@ void read_boot(DOS_FS * fs)
     if (verbose)
 	printf("Checking we can access the last sector of the filesystem\n");
     /* Can't access last odd sector anyway, so round down */
     fs_test((off_t)((total_sectors & ~1) - 1) * logical_sector_size,
 	    logical_sector_size);
+
     fat_length = le16toh(b.fat_length) ?
 	le16toh(b.fat_length) : le32toh(b.fat32_length);
+    if (!fat_length)
+	die("FAT size is zero.");
+
     fs->fat_start = (off_t)le16toh(b.reserved) * logical_sector_size;
     fs->root_start = ((off_t)le16toh(b.reserved) + b.fats * fat_length) *
 	logical_sector_size;
     fs->root_entries = GET_UNALIGNED_W(b.dir_entries);
     fs->data_start = fs->root_start + ROUND_TO_MULTIPLE(fs->root_entries <<
 							MSDOS_DIR_BITS,
 							logical_sector_size);
+
     data_size = (off_t)total_sectors * logical_sector_size - fs->data_start;
+    if (data_size < fs->cluster_size)
+	die("Filesystem has no space for any data clusters");
+
     fs->data_clusters = data_size / fs->cluster_size;
     fs->root_cluster = 0;	/* indicates standard, pre-FAT32 root dir */
     fs->fsinfo_start = 0;	/* no FSINFO structure */
     fs->free_clusters = -1;	/* unknown */
     if (!b.fat_length && b.fat32_length) {
diff --git a/src/fsck.fat.h b/src/fsck.fat.h
index d150ab9..168049e 100644
--- a/src/fsck.fat.h
+++ b/src/fsck.fat.h
@@ -150,11 +150,11 @@ typedef struct {
 } FAT_ENTRY;
 
 typedef struct {
     int nfats;
     off_t fat_start;
-    unsigned int fat_size;	/* unit is bytes */
+    off_t fat_size;		/* unit is bytes */
     unsigned int fat_bits;	/* size of a FAT entry */
     unsigned int eff_fat_bits;	/* # of used bits in a FAT entry */
     uint32_t root_cluster;	/* 0 for old-style root dir */
     off_t root_start;
     unsigned int root_entries;

Vulnerable code:     printf("%10d bytes per FAT (= %u sectors)\n", fs->fat_size,
	   fs->fat_size / lss);
    unsigned fat_length;
    unsigned int fat_size;	/* unit is bytes */
Fixed code:     printf("%10lld bytes per FAT (= %llu sectors)\n", (long long)fs->fat_size,
	   (long long)fs->fat_size / lss);
    off_t fat_length;

    if (!fat_length)
	die("FAT size is zero.");


    if (data_size < fs->cluster_size)
	die("Filesystem has no space for any data clusters");

    off_t fat_size;		/* unit is bytes */