From f30f9339c9a0b9bbcc6f5ad38804af39db1f479e Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Wed, 19 Sep 2018 18:26:01 +0200 Subject: Windows: fix low severity vulnerability in driver that allowed reading 3 bytes of kernel stack memory (with a rare possibility of 25 additional bytes). Reported by Tim Harrison. --- src/Driver/Ntdriver.c | 131 +++++++++++++++++++++++++++++++------------------- 1 file changed, 82 insertions(+), 49 deletions(-) (limited to 'src/Driver/Ntdriver.c') diff --git a/src/Driver/Ntdriver.c b/src/Driver/Ntdriver.c index 5243b34c..cea48b27 100644 --- a/src/Driver/Ntdriver.c +++ b/src/Driver/Ntdriver.c @@ -1902,11 +1902,24 @@ NTSTATUS ProcessMainDeviceControlIrp (PDEVICE_OBJECT DeviceObject, PEXTENSION Ex UNICODE_STRING FullFileName; IO_STATUS_BLOCK IoStatus; LARGE_INTEGER offset; - byte readBuffer [TC_SECTOR_SIZE_BIOS]; + size_t devicePathLen = 0; if (!ValidateIOBufferSize (Irp, sizeof (GetSystemDriveConfigurationRequest), ValidateInputOutput)) break; + // check that request->DevicePath has the expected format "\\Device\\HarddiskXXX\\Partition0" + if ( !NT_SUCCESS (RtlUnalignedStringCchLengthW (request->DevicePath, TC_MAX_PATH, &devicePathLen)) + || (devicePathLen < 28) // 28 is the length of "\\Device\\Harddisk0\\Partition0" which is the minimum + || (devicePathLen > 30) // 30 is the length of "\\Device\\Harddisk255\\Partition0" which is the maximum + || (memcmp (request->DevicePath, L"\\Device\\Harddisk", 16 * sizeof (WCHAR))) + || (memcmp (&request->DevicePath[devicePathLen - 11], L"\\Partition0", 11 * sizeof (WCHAR))) + ) + { + Irp->IoStatus.Status = STATUS_INVALID_PARAMETER; + Irp->IoStatus.Information = 0; + break; + } + EnsureNullTerminatedString (request->DevicePath, sizeof (request->DevicePath)); RtlInitUnicodeString (&FullFileName, request->DevicePath); @@ -1918,68 +1931,88 @@ NTSTATUS ProcessMainDeviceControlIrp (PDEVICE_OBJECT DeviceObject, PEXTENSION Ex if (NT_SUCCESS (ntStatus)) { - // Determine if the first sector contains a portion of the VeraCrypt Boot Loader - offset.QuadPart = 0; // MBR - - ntStatus = ZwReadFile (NtFileHandle, - NULL, - NULL, - NULL, - &IoStatus, - readBuffer, - sizeof(readBuffer), - &offset, - NULL); - - if (NT_SUCCESS (ntStatus)) + byte *readBuffer = TCalloc (TC_MAX_VOLUME_SECTOR_SIZE); + if (!readBuffer) { - size_t i; + Irp->IoStatus.Status = STATUS_INSUFFICIENT_RESOURCES; + Irp->IoStatus.Information = 0; + } + else + { + // Determine if the first sector contains a portion of the VeraCrypt Boot Loader + offset.QuadPart = 0; // MBR - // Check for dynamic drive - request->DriveIsDynamic = FALSE; + ntStatus = ZwReadFile (NtFileHandle, + NULL, + NULL, + NULL, + &IoStatus, + readBuffer, + TC_MAX_VOLUME_SECTOR_SIZE, + &offset, + NULL); - if (readBuffer[510] == 0x55 && readBuffer[511] == 0xaa) + if (NT_SUCCESS (ntStatus)) { - int i; - for (i = 0; i < 4; ++i) + // check that we could read all needed data + if (IoStatus.Information >= TC_SECTOR_SIZE_BIOS) { - if (readBuffer[446 + i * 16 + 4] == PARTITION_LDM) + size_t i; + + // Check for dynamic drive + request->DriveIsDynamic = FALSE; + + if (readBuffer[510] == 0x55 && readBuffer[511] == 0xaa) { - request->DriveIsDynamic = TRUE; - break; + int i; + for (i = 0; i < 4; ++i) + { + if (readBuffer[446 + i * 16 + 4] == PARTITION_LDM) + { + request->DriveIsDynamic = TRUE; + break; + } + } } - } - } - request->BootLoaderVersion = 0; - request->Configuration = 0; - request->UserConfiguration = 0; - request->CustomUserMessage[0] = 0; + request->BootLoaderVersion = 0; + request->Configuration = 0; + request->UserConfiguration = 0; + request->CustomUserMessage[0] = 0; - // Search for the string "VeraCrypt" - for (i = 0; i < sizeof (readBuffer) - strlen (TC_APP_NAME); ++i) - { - if (memcmp (readBuffer + i, TC_APP_NAME, strlen (TC_APP_NAME)) == 0) - { - request->BootLoaderVersion = BE16 (*(uint16 *) (readBuffer + TC_BOOT_SECTOR_VERSION_OFFSET)); - request->Configuration = readBuffer[TC_BOOT_SECTOR_CONFIG_OFFSET]; - - if (request->BootLoaderVersion != 0 && request->BootLoaderVersion <= VERSION_NUM) + // Search for the string "VeraCrypt" + for (i = 0; i < sizeof (readBuffer) - strlen (TC_APP_NAME); ++i) { - request->UserConfiguration = readBuffer[TC_BOOT_SECTOR_USER_CONFIG_OFFSET]; - memcpy (request->CustomUserMessage, readBuffer + TC_BOOT_SECTOR_USER_MESSAGE_OFFSET, TC_BOOT_SECTOR_USER_MESSAGE_MAX_LENGTH); + if (memcmp (readBuffer + i, TC_APP_NAME, strlen (TC_APP_NAME)) == 0) + { + request->BootLoaderVersion = BE16 (*(uint16 *) (readBuffer + TC_BOOT_SECTOR_VERSION_OFFSET)); + request->Configuration = readBuffer[TC_BOOT_SECTOR_CONFIG_OFFSET]; + + if (request->BootLoaderVersion != 0 && request->BootLoaderVersion <= VERSION_NUM) + { + request->UserConfiguration = readBuffer[TC_BOOT_SECTOR_USER_CONFIG_OFFSET]; + memcpy (request->CustomUserMessage, readBuffer + TC_BOOT_SECTOR_USER_MESSAGE_OFFSET, TC_BOOT_SECTOR_USER_MESSAGE_MAX_LENGTH); + } + break; + } } - break; + + Irp->IoStatus.Status = STATUS_SUCCESS; + Irp->IoStatus.Information = sizeof (*request); + } + else + { + Irp->IoStatus.Status = STATUS_INVALID_PARAMETER; + Irp->IoStatus.Information = 0; } } + else + { + Irp->IoStatus.Status = ntStatus; + Irp->IoStatus.Information = 0; + } - Irp->IoStatus.Status = STATUS_SUCCESS; - Irp->IoStatus.Information = sizeof (*request); - } - else - { - Irp->IoStatus.Status = ntStatus; - Irp->IoStatus.Information = 0; + TCfree (readBuffer); } ZwClose (NtFileHandle); -- cgit v1.2.3