From ce72be65bbf633ac6e2179d4b683101547f6e537 Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Sat, 3 Oct 2026 23:51:36 +0900 Subject: [PATCH] Unix: avoid unaligned FAT field accesses The FAT formatter stored multi-byte boot sector and FSInfo fields by casting positions in the sector buffer to integer pointers, and GetMaxHiddenVolumeSize read the outer volume's boot sector the same way. Several FAT fields start at odd offsets (bytes per sector at 11, root directory entry count at 17, total sectors at 19, and the volume ID at 39 or 67), so these accesses were misaligned. That is undefined behavior whatever the byte order, and it can fault on targets that require aligned accesses. An alignment-sanitized build reported the stores in PutBoot and, for FAT12 and FAT16 outer volumes, the read of the root directory entry count. Copy each field between the buffer and a local integer with memcpy, keeping the Endian::Little conversions, and copy the volume ID as its four random bytes. Fields at even offsets use the same helpers: they were aligned only because the sector buffer comes from malloc. The FAT scan for the last used cluster is unchanged; it reads 32-bit words at multiples of four in a malloc'd buffer and only tests them for zero. Validated with x86_64 builds using -fsanitize=alignment and big-endian s390x builds under qemu. With fixed volume ID bytes, the formatter output for 18 FAT12, FAT16 and FAT32 layouts, including the FAT32 backup boot sector, is byte-identical before and after. The hidden volume size of 16 volumes created and populated through the CLI is unchanged and matches a separate implementation; the s390x checks used the big-endian crypto fixes from #1899 to open the volumes. The sanitizer and trap builds no longer report these accesses. --- src/Core/CoreBase.cpp | 23 +++++++++++++++++++---- src/Core/FatFormatter.cpp | 39 ++++++++++++++++++++++++++------------- 2 files changed, 45 insertions(+), 17 deletions(-) diff --git a/src/Core/CoreBase.cpp b/src/Core/CoreBase.cpp index e82a7f4f..8483f239 100644 --- a/src/Core/CoreBase.cpp +++ b/src/Core/CoreBase.cpp @@ -140,6 +140,21 @@ namespace VeraCrypt #endif } + // FAT boot sector fields are not necessarily aligned (some start at odd offsets), so they must not be accessed through integer pointers + static uint16 GetLittleEndian16 (const uint8 *src) + { + uint16 value; + memcpy (&value, src, sizeof (value)); + return Endian::Little (value); + } + + static uint32 GetLittleEndian32 (const uint8 *src) + { + uint32 value; + memcpy (&value, src, sizeof (value)); + return Endian::Little (value); + } + uint64 CoreBase::GetMaxHiddenVolumeSize (shared_ptr outerVolume) const { uint32 sectorSize = outerVolume->GetSectorSize(); @@ -160,21 +175,21 @@ namespace VeraCrypt throw ParameterIncorrect (SRC_POS); uint32 clusterSize = bootSector[13] * sectorSize; - uint32 reservedSectorCount = Endian::Little (*(uint16 *) (bootSector + 14)); + uint32 reservedSectorCount = GetLittleEndian16 (bootSector + 14); uint32 fatCount = bootSector[16]; uint64 fatSectorCount; if (fatType == 32) - fatSectorCount = Endian::Little (*(uint32 *) (bootSector + 36)); + fatSectorCount = GetLittleEndian32 (bootSector + 36); else - fatSectorCount = Endian::Little (*(uint16 *) (bootSector + 22)); + fatSectorCount = GetLittleEndian16 (bootSector + 22); uint64 fatSize = fatSectorCount * sectorSize; uint64 fatStartOffset = reservedSectorCount * sectorSize; uint64 dataAreaOffset = reservedSectorCount * sectorSize + fatSize * fatCount; if (fatType < 32) - dataAreaOffset += Endian::Little (*(uint16 *) (bootSector + 17)) * 32; + dataAreaOffset += GetLittleEndian16 (bootSector + 17) * 32; SecureBuffer sector (sectorSize); diff --git a/src/Core/FatFormatter.cpp b/src/Core/FatFormatter.cpp index 636be527..28f1b9f4 100644 --- a/src/Core/FatFormatter.cpp +++ b/src/Core/FatFormatter.cpp @@ -149,6 +149,19 @@ namespace VeraCrypt } } + // FAT fields are not necessarily aligned (some start at odd offsets), so they must not be accessed through integer pointers + static void PutLittleEndian16 (uint8 *dest, uint16 value) + { + value = Endian::Little (value); + memcpy (dest, &value, sizeof (value)); + } + + static void PutLittleEndian32 (uint8 *dest, uint32 value) + { + value = Endian::Little (value); + memcpy (dest, &value, sizeof (value)); + } + static void PutBoot (fatparams * ft, uint8 *boot, uint32 volumeId) { int cnt = 0; @@ -158,10 +171,10 @@ namespace VeraCrypt boot[cnt++] = 0x90; memcpy (boot + cnt, "MSDOS5.0", 8); /* system id */ cnt += 8; - *(int16 *)(boot + cnt) = Endian::Little (ft->sector_size); /* bytes per sector */ + PutLittleEndian16 (boot + cnt, ft->sector_size); /* bytes per sector */ cnt += 2; boot[cnt++] = (int8) ft->cluster_size; /* sectors per cluster */ - *(int16 *)(boot + cnt) = Endian::Little (ft->reserved); /* reserved sectors */ + PutLittleEndian16 (boot + cnt, ft->reserved); /* reserved sectors */ cnt += 2; boot[cnt++] = (int8) ft->fats; /* 2 fats */ @@ -172,11 +185,11 @@ namespace VeraCrypt } else { - *(int16 *)(boot + cnt) = Endian::Little (ft->dir_entries); /* 512 root entries */ + PutLittleEndian16 (boot + cnt, ft->dir_entries); /* 512 root entries */ cnt += 2; } - *(int16 *)(boot + cnt) = Endian::Little (ft->sectors); /* # sectors */ + PutLittleEndian16 (boot + cnt, ft->sectors); /* # sectors */ cnt += 2; boot[cnt++] = (int8) ft->media; /* media byte */ @@ -187,22 +200,22 @@ namespace VeraCrypt } else { - *(uint16 *)(boot + cnt) = Endian::Little ((uint16) ft->fat_length); /* fat size */ + PutLittleEndian16 (boot + cnt, (uint16) ft->fat_length); /* fat size */ cnt += 2; } - *(int16 *)(boot + cnt) = Endian::Little (ft->secs_track); /* # sectors per track */ + PutLittleEndian16 (boot + cnt, ft->secs_track); /* # sectors per track */ cnt += 2; - *(int16 *)(boot + cnt) = Endian::Little (ft->heads); /* # heads */ + PutLittleEndian16 (boot + cnt, ft->heads); /* # heads */ cnt += 2; - *(int32 *)(boot + cnt) = Endian::Little (ft->hidden); /* # hidden sectors */ + PutLittleEndian32 (boot + cnt, ft->hidden); /* # hidden sectors */ cnt += 4; - *(int32 *)(boot + cnt) = Endian::Little (ft->total_sect); /* # huge sectors */ + PutLittleEndian32 (boot + cnt, ft->total_sect); /* # huge sectors */ cnt += 4; if(ft->size_fat == 32) { - *(int32 *)(boot + cnt) = Endian::Little (ft->fat_length); cnt += 4; /* fat size 32 */ + PutLittleEndian32 (boot + cnt, ft->fat_length); cnt += 4; /* fat size 32 */ boot[cnt++] = 0x00; /* ExtFlags */ boot[cnt++] = 0x00; boot[cnt++] = 0x00; /* FSVer */ @@ -222,7 +235,7 @@ namespace VeraCrypt boot[cnt++] = 0x00; /* reserved */ boot[cnt++] = 0x29; /* boot sig */ - *(int32 *)(boot + cnt) = volumeId; + memcpy (boot + cnt, &volumeId, sizeof (volumeId)); cnt += 4; memcpy (boot + cnt, ft->volume_name, 11); /* vol title */ @@ -257,10 +270,10 @@ namespace VeraCrypt sector[484+0] = 0x72; // Free cluster count - *(uint32 *)(sector + 488) = Endian::Little (ft->cluster_count - ft->size_root_dir / ft->sector_size / ft->cluster_size); + PutLittleEndian32 (sector + 488, ft->cluster_count - ft->size_root_dir / ft->sector_size / ft->cluster_size); // Next free cluster - *(uint32 *)(sector + 492) = Endian::Little ((uint32) 2); + PutLittleEndian32 (sector + 492, 2); sector[508+3] = 0xaa; /* TrailSig */ sector[508+2] = 0x55;