[FSTUB] Fix out of bounds access in IoReadDiskSignature

- Convert PARTITION_TABLE_OFFSET to the number of bytes instead of
  (number of bytes) / 2. This avoids many confusing casts
- Use a cache aligned buffer for MBR
This commit is contained in:
Victor Perevertkin 2020-12-07 14:43:34 +03:00
parent 5d4ece342c
commit 8d2fe54188
No known key found for this signature in database
GPG key ID: C750B7222E9C7830
3 changed files with 22 additions and 28 deletions

View file

@ -1712,8 +1712,7 @@ xHalExamineMBR(IN PDEVICE_OBJECT DeviceObject,
} }
/* Get the partition entry */ /* Get the partition entry */
PartitionDescriptor = (PPARTITION_DESCRIPTOR) PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET];
&(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]);
/* Make sure it's what the caller wanted */ /* Make sure it's what the caller wanted */
if (PartitionDescriptor->PartitionType != MbrTypeIdentifier) if (PartitionDescriptor->PartitionType != MbrTypeIdentifier)
@ -1831,7 +1830,7 @@ xHalIoReadPartitionTable(IN PDEVICE_OBJECT DeviceObject,
DiskGeometryEx.DiskSize, MaxSector); DiskGeometryEx.DiskSize, MaxSector);
/* Allocate our buffer */ /* Allocate our buffer */
Buffer = ExAllocatePoolWithTag(NonPagedPool, InputSize, TAG_FILE_SYSTEM); Buffer = ExAllocatePoolWithTag(NonPagedPoolCacheAligned, InputSize, TAG_FILE_SYSTEM);
if (!Buffer) if (!Buffer)
{ {
/* Fail, free the input buffer */ /* Fail, free the input buffer */
@ -1901,12 +1900,11 @@ xHalIoReadPartitionTable(IN PDEVICE_OBJECT DeviceObject,
if (!Offset.QuadPart) if (!Offset.QuadPart)
{ {
/* Then read the signature off the disk */ /* Then read the signature off the disk */
(*PartitionBuffer)->Signature = ((PULONG)Buffer)[PARTITION_TABLE_OFFSET / 2 - 1]; (*PartitionBuffer)->Signature = *(PUINT32)&Buffer[DISK_SIGNATURE_OFFSET];
} }
/* Get the partition descriptor array */ /* Get the partition descriptor array */
PartitionDescriptor = (PPARTITION_DESCRIPTOR) PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET];
&(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]);
/* Start looping partitions */ /* Start looping partitions */
j++; j++;
@ -2091,8 +2089,7 @@ xHalIoReadPartitionTable(IN PDEVICE_OBJECT DeviceObject,
Offset.QuadPart = 0; Offset.QuadPart = 0;
/* Go back to the descriptor array and loop it */ /* Go back to the descriptor array and loop it */
PartitionDescriptor = (PPARTITION_DESCRIPTOR) PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET];
&(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]);
for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++) for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++)
{ {
/* Check if this is a container partition, since we skipped them */ /* Check if this is a container partition, since we skipped them */
@ -2237,7 +2234,7 @@ xHalIoSetPartitionInformation(IN PDEVICE_OBJECT DeviceObject,
} }
/* Allocate our partition buffer */ /* Allocate our partition buffer */
Buffer = ExAllocatePoolWithTag(NonPagedPool, PAGE_SIZE, TAG_FILE_SYSTEM); Buffer = ExAllocatePoolWithTag(NonPagedPoolCacheAligned, PAGE_SIZE, TAG_FILE_SYSTEM);
if (!Buffer) return STATUS_INSUFFICIENT_RESOURCES; if (!Buffer) return STATUS_INSUFFICIENT_RESOURCES;
/* Initialize the event we'll use and loop partitions */ /* Initialize the event we'll use and loop partitions */
@ -2290,8 +2287,7 @@ xHalIoSetPartitionInformation(IN PDEVICE_OBJECT DeviceObject,
} }
/* Get the partition descriptors and loop them */ /* Get the partition descriptors and loop them */
PartitionDescriptor = (PPARTITION_DESCRIPTOR) PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET];
&(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]);
for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++) for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++)
{ {
/* Check if it's unused or a container partition */ /* Check if it's unused or a container partition */
@ -2352,8 +2348,7 @@ xHalIoSetPartitionInformation(IN PDEVICE_OBJECT DeviceObject,
if (Entry <= NUM_PARTITION_TABLE_ENTRIES) break; if (Entry <= NUM_PARTITION_TABLE_ENTRIES) break;
/* Nothing found yet, get the partition array again */ /* Nothing found yet, get the partition array again */
PartitionDescriptor = (PPARTITION_DESCRIPTOR) PartitionDescriptor = (PPARTITION_DESCRIPTOR)&Buffer[PARTITION_TABLE_OFFSET];
&(((PUSHORT)Buffer)[PARTITION_TABLE_OFFSET]);
for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++) for (Entry = 1; Entry <= NUM_PARTITION_TABLE_ENTRIES; Entry++, PartitionDescriptor++)
{ {
/* Check if this was a container partition (we skipped these) */ /* Check if this was a container partition (we skipped these) */
@ -2469,7 +2464,7 @@ xHalIoWritePartitionTable(IN PDEVICE_OBJECT DeviceObject,
DiskLayout->TableCount = (PartitionBuffer->PartitionCount + NUM_PARTITION_TABLE_ENTRIES - 1) / NUM_PARTITION_TABLE_ENTRIES; DiskLayout->TableCount = (PartitionBuffer->PartitionCount + NUM_PARTITION_TABLE_ENTRIES - 1) / NUM_PARTITION_TABLE_ENTRIES;
/* Allocate our partition buffer */ /* Allocate our partition buffer */
Buffer = ExAllocatePoolWithTag(NonPagedPool, PAGE_SIZE, TAG_FILE_SYSTEM); Buffer = ExAllocatePoolWithTag(NonPagedPoolCacheAligned, PAGE_SIZE, TAG_FILE_SYSTEM);
if (!Buffer) return STATUS_INSUFFICIENT_RESOURCES; if (!Buffer) return STATUS_INSUFFICIENT_RESOURCES;
/* Loop the entries */ /* Loop the entries */
@ -2520,7 +2515,7 @@ xHalIoWritePartitionTable(IN PDEVICE_OBJECT DeviceObject,
if (!IsSuperFloppy) if (!IsSuperFloppy)
{ {
/* Set the boot record signature */ /* Set the boot record signature */
Buffer[BOOT_SIGNATURE_OFFSET] = BOOT_RECORD_SIGNATURE; ((PUSHORT)Buffer)[BOOT_SIGNATURE_OFFSET] = BOOT_RECORD_SIGNATURE;
/* By default, don't require a rewrite */ /* By default, don't require a rewrite */
DoRewrite = FALSE; DoRewrite = FALSE;
@ -2529,12 +2524,10 @@ xHalIoWritePartitionTable(IN PDEVICE_OBJECT DeviceObject,
if (!Offset.QuadPart) if (!Offset.QuadPart)
{ {
/* Check if the signature doesn't match */ /* Check if the signature doesn't match */
if (((PULONG)Buffer)[PARTITION_TABLE_OFFSET / 2 - 1] != if (*(PUINT32)&Buffer[PARTITION_TABLE_OFFSET] != PartitionBuffer->Signature)
PartitionBuffer->Signature)
{ {
/* Then write the signature and now we need a rewrite */ /* Then write the signature and now we need a rewrite */
((PULONG)Buffer)[PARTITION_TABLE_OFFSET / 2 - 1] = *(PUINT32)&Buffer[PARTITION_TABLE_OFFSET] = PartitionBuffer->Signature;
PartitionBuffer->Signature;
DoRewrite = TRUE; DoRewrite = TRUE;
} }
} }

View file

@ -674,7 +674,7 @@ FstubDetectPartitionStyle(IN PDISK_INFORMATION Disk,
/* Get the partition descriptor array */ /* Get the partition descriptor array */
PartitionDescriptor = (PPARTITION_DESCRIPTOR) PartitionDescriptor = (PPARTITION_DESCRIPTOR)
&(Disk->Buffer[PARTITION_TABLE_OFFSET]); &(Disk->Buffer[PARTITION_TABLE_OFFSET / sizeof(Disk->Buffer[0])]);
/* If we have not the 0xAA55 then it's raw partition */ /* If we have not the 0xAA55 then it's raw partition */
if (Disk->Buffer[BOOT_SIGNATURE_OFFSET] != BOOT_RECORD_SIGNATURE) if (Disk->Buffer[BOOT_SIGNATURE_OFFSET] != BOOT_RECORD_SIGNATURE)
{ {
@ -2140,7 +2140,7 @@ IoReadDiskSignature(IN PDEVICE_OBJECT DeviceObject,
IN ULONG BytesPerSector, IN ULONG BytesPerSector,
OUT PDISK_SIGNATURE Signature) OUT PDISK_SIGNATURE Signature)
{ {
PULONG Buffer; PUCHAR Buffer;
NTSTATUS Status; NTSTATUS Status;
ULONG HeaderCRC32, i, CheckSum; ULONG HeaderCRC32, i, CheckSum;
PEFI_PARTITION_HEADER EFIHeader; PEFI_PARTITION_HEADER EFIHeader;
@ -2207,7 +2207,7 @@ IoReadDiskSignature(IN PDEVICE_OBJECT DeviceObject,
/* Then zero the one in EFI header. This is needed to compute header checksum */ /* Then zero the one in EFI header. This is needed to compute header checksum */
EFIHeader->HeaderCRC32 = 0; EFIHeader->HeaderCRC32 = 0;
/* Compute header checksum and compare with the one present in partition table */ /* Compute header checksum and compare with the one present in partition table */
if (RtlComputeCrc32(0, (PUCHAR)Buffer, sizeof(EFI_PARTITION_HEADER)) != HeaderCRC32) if (RtlComputeCrc32(0, Buffer, sizeof(EFI_PARTITION_HEADER)) != HeaderCRC32)
{ {
Status = STATUS_DISK_CORRUPT_ERROR; Status = STATUS_DISK_CORRUPT_ERROR;
goto Cleanup; goto Cleanup;
@ -2220,14 +2220,14 @@ IoReadDiskSignature(IN PDEVICE_OBJECT DeviceObject,
else else
{ {
/* Compute MBR checksum */ /* Compute MBR checksum */
for (i = 0, CheckSum = 0; i < 512 / sizeof(ULONG) ; i++) for (i = 0, CheckSum = 0; i < BytesPerSector / sizeof(UINT32); i++)
{ {
CheckSum += Buffer[i]; CheckSum += *(PUINT32)&Buffer[i];
} }
/* Set partition table style to MBR and return signature (offset 440) and checksum */ /* Set partition table style to MBR and return signature (offset 440) and checksum */
Signature->PartitionStyle = PARTITION_STYLE_MBR; Signature->PartitionStyle = PARTITION_STYLE_MBR;
Signature->Mbr.Signature = Buffer[PARTITION_TABLE_OFFSET / 2 - 1]; Signature->Mbr.Signature = *(PUINT32)&Buffer[DISK_SIGNATURE_OFFSET];
Signature->Mbr.CheckSum = CheckSum; Signature->Mbr.CheckSum = CheckSum;
} }

View file

@ -240,9 +240,10 @@ xKdUnmapVirtualAddress(
// //
// Various offsets in the boot record // Various offsets in the boot record
// //
#define PARTITION_TABLE_OFFSET (0x1BE / 2) #define DISK_SIGNATURE_OFFSET 0x1B8
#define BOOT_SIGNATURE_OFFSET ((0x200 / 2) - 1) #define PARTITION_TABLE_OFFSET 0x1BE
#define BOOT_RECORD_RESERVED 0x1BC #define BOOT_SIGNATURE_OFFSET ((0x200 / sizeof(INT16)) - 1)
#define BOOT_RECORD_SIGNATURE 0xAA55 #define BOOT_RECORD_SIGNATURE 0xAA55
#define NUM_PARTITION_TABLE_ENTRIES 4 #define NUM_PARTITION_TABLE_ENTRIES 4