From 3137d36d9a29ed55be5837abf1be3f959f831abc Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Mon, 14 Jul 2014 16:59:14 +0200 Subject: Static Code Analysis : Use Safe string functions inside VeraCrypt Device Driver to avoid potential security issues. Add many checks for NULL pointers to handle low memory use cases. --- src/Driver/EncryptedIoQueue.c | 14 ++++++++++ src/Driver/Ntdriver.c | 62 +++++++++++++++++++++---------------------- src/Driver/Ntdriver.h | 4 +-- src/Driver/Ntvol.c | 20 ++++++++++---- 4 files changed, 62 insertions(+), 38 deletions(-) diff --git a/src/Driver/EncryptedIoQueue.c b/src/Driver/EncryptedIoQueue.c index 7cf48cd6..bb76a0f5 100644 --- a/src/Driver/EncryptedIoQueue.c +++ b/src/Driver/EncryptedIoQueue.c @@ -510,6 +510,15 @@ static VOID MainThreadProc (PVOID threadArg) KeWaitForSingleObject (&queue->QueueResumedEvent, Executive, KernelMode, FALSE, NULL); item = GetPoolBuffer (queue, sizeof (EncryptedIoQueueItem)); + if (!item) + { + TCCompleteDiskIrp (irp, STATUS_INSUFFICIENT_RESOURCES, 0); + DecrementOutstandingIoCount (queue); + IoReleaseRemoveLock (&queue->RemoveLock, irp); + + continue; + } + item->Queue = queue; item->OriginalIrp = irp; item->Status = STATUS_SUCCESS; @@ -687,6 +696,11 @@ static VOID MainThreadProc (PVOID threadArg) // Create IO request request = GetPoolBuffer (queue, sizeof (EncryptedIoRequest)); + if (!request) + { + CompleteOriginalIrp (item, STATUS_INSUFFICIENT_RESOURCES, 0); + break; + } request->Item = item; request->CompleteOriginalIrp = isLastFragment; request->Offset = fragmentOffset; diff --git a/src/Driver/Ntdriver.c b/src/Driver/Ntdriver.c index 2456383b..9574483b 100644 --- a/src/Driver/Ntdriver.c +++ b/src/Driver/Ntdriver.c @@ -33,6 +33,8 @@ #include #include +#include + /* Init section, which is thrown away as soon as DriverEntry returns */ #pragma alloc_text(INIT,DriverEntry) #pragma alloc_text(INIT,TCCreateRootDeviceObject) @@ -359,8 +361,8 @@ NTSTATUS TCCreateRootDeviceObject (PDRIVER_OBJECT DriverObject) Dump ("TCCreateRootDeviceObject BEGIN\n"); ASSERT (KeGetCurrentIrql() == PASSIVE_LEVEL); - wcscpy (dosname, (LPWSTR) DOS_ROOT_PREFIX); - wcscpy (ntname, (LPWSTR) NT_ROOT_PREFIX); + RtlStringCbCopyW (dosname, sizeof(dosname),(LPWSTR) DOS_ROOT_PREFIX); + RtlStringCbCopyW (ntname, sizeof(ntname),(LPWSTR) NT_ROOT_PREFIX); RtlInitUnicodeString (&ntUnicodeString, ntname); RtlInitUnicodeString (&Win32NameString, dosname); @@ -419,8 +421,8 @@ NTSTATUS TCCreateDeviceObject (PDRIVER_OBJECT DriverObject, Dump ("TCCreateDeviceObject BEGIN\n"); ASSERT (KeGetCurrentIrql() == PASSIVE_LEVEL); - TCGetDosNameFromNumber (dosname, mount->nDosDriveNo); - TCGetNTNameFromNumber (ntname, mount->nDosDriveNo); + TCGetDosNameFromNumber (dosname, sizeof(dosname),mount->nDosDriveNo); + TCGetNTNameFromNumber (ntname, sizeof(ntname),mount->nDosDriveNo); RtlInitUnicodeString (&ntUnicodeString, ntname); RtlInitUnicodeString (&Win32NameString, dosname); @@ -510,7 +512,7 @@ NTSTATUS ProcessVolumeDeviceControlIrp (PDEVICE_OBJECT DeviceObject, PEXTENSION WCHAR ntName[256]; PMOUNTDEV_NAME outputBuffer = (PMOUNTDEV_NAME) Irp->AssociatedIrp.SystemBuffer; - TCGetNTNameFromNumber (ntName, Extension->nDosDriveNo); + TCGetNTNameFromNumber (ntName, sizeof(ntName),Extension->nDosDriveNo); RtlInitUnicodeString (&ntUnicodeString, ntName); outputBuffer->NameLength = ntUnicodeString.Length; @@ -545,9 +547,9 @@ NTSTATUS ProcessVolumeDeviceControlIrp (PDEVICE_OBJECT DeviceObject, PEXTENSION UCHAR volId[128], tmp[] = { 0,0 }; PMOUNTDEV_UNIQUE_ID outputBuffer = (PMOUNTDEV_UNIQUE_ID) Irp->AssociatedIrp.SystemBuffer; - strcpy (volId, TC_UNIQUE_ID_PREFIX); + RtlStringCbCopyA (volId, sizeof(volId),TC_UNIQUE_ID_PREFIX); tmp[0] = 'A' + (UCHAR) Extension->nDosDriveNo; - strcat (volId, tmp); + RtlStringCbCatA (volId, sizeof(volId),tmp); outputBuffer->UniqueIdLength = (USHORT) strlen (volId); outLength = (ULONG) (strlen (volId) + sizeof (USHORT)); @@ -582,7 +584,7 @@ NTSTATUS ProcessVolumeDeviceControlIrp (PDEVICE_OBJECT DeviceObject, PEXTENSION break; } - TCGetDosNameFromNumber (ntName, Extension->nDosDriveNo); + TCGetDosNameFromNumber (ntName, sizeof(ntName),Extension->nDosDriveNo); RtlInitUnicodeString (&ntUnicodeString, ntName); outLength = FIELD_OFFSET(MOUNTDEV_SUGGESTED_LINK_NAME,Name) + ntUnicodeString.Length; @@ -1122,7 +1124,7 @@ NTSTATUS ProcessMainDeviceControlIrp (PDEVICE_OBJECT DeviceObject, PEXTENSION Ex if (IsVolumeAccessibleByCurrentUser (ListExtension)) { list->ulMountedDrives |= (1 << ListExtension->nDosDriveNo); - wcscpy (list->wszVolume[ListExtension->nDosDriveNo], ListExtension->wszVolume); + RtlStringCbCopyW (list->wszVolume[ListExtension->nDosDriveNo], sizeof(list->wszVolume[ListExtension->nDosDriveNo]),ListExtension->wszVolume); list->diskLength[ListExtension->nDosDriveNo] = ListExtension->DiskLength; list->ea[ListExtension->nDosDriveNo] = ListExtension->cryptoInfo->ea; if (ListExtension->cryptoInfo->hiddenVolume) @@ -1171,7 +1173,7 @@ NTSTATUS ProcessMainDeviceControlIrp (PDEVICE_OBJECT DeviceObject, PEXTENSION Ex if (IsVolumeAccessibleByCurrentUser (ListExtension)) { prop->uniqueId = ListExtension->UniqueVolumeId; - wcscpy (prop->wszVolume, ListExtension->wszVolume); + RtlStringCbCopyW (prop->wszVolume, sizeof(prop->wszVolume),ListExtension->wszVolume); prop->diskLength = ListExtension->DiskLength; prop->ea = ListExtension->cryptoInfo->ea; prop->mode = ListExtension->cryptoInfo->mode; @@ -1747,16 +1749,14 @@ VOID VolumeThreadProc (PVOID Context) if (memcmp (pThreadBlock->mount->wszVolume, WIDE ("\\Device"), 14) != 0) { - wcscpy (pThreadBlock->wszMountVolume, WIDE ("\\??\\")); - wcsncat (pThreadBlock->wszMountVolume, pThreadBlock->mount->wszVolume, - sizeof (pThreadBlock->wszMountVolume) / 2 - 5); + RtlStringCbCopyW (pThreadBlock->wszMountVolume, sizeof(pThreadBlock->wszMountVolume),WIDE ("\\??\\")); + RtlStringCbCatW (pThreadBlock->wszMountVolume, sizeof(pThreadBlock->wszMountVolume),pThreadBlock->mount->wszVolume); bDevice = FALSE; } else { pThreadBlock->wszMountVolume[0] = 0; - wcsncat (pThreadBlock->wszMountVolume, pThreadBlock->mount->wszVolume, - sizeof (pThreadBlock->wszMountVolume) / 2 - 1); + RtlStringCbCatW (pThreadBlock->wszMountVolume, sizeof(pThreadBlock->wszMountVolume),pThreadBlock->mount->wszVolume); bDevice = TRUE; } @@ -1838,26 +1838,26 @@ VOID VolumeThreadProc (PVOID Context) } } -void TCGetNTNameFromNumber (LPWSTR ntname, int nDriveNo) +void TCGetNTNameFromNumber (LPWSTR ntname, int cbNtName, int nDriveNo) { WCHAR tmp[3] = {0, ':', 0}; int j = nDriveNo + (WCHAR) 'A'; tmp[0] = (short) j; - wcscpy (ntname, (LPWSTR) NT_MOUNT_PREFIX); - wcsncat (ntname, tmp, 1); + RtlStringCbCopyW (ntname, cbNtName,(LPWSTR) NT_MOUNT_PREFIX); + RtlStringCbCatW (ntname, cbNtName, tmp); } -void TCGetDosNameFromNumber (LPWSTR dosname, int nDriveNo) +void TCGetDosNameFromNumber (LPWSTR dosname,int cbDosName, int nDriveNo) { WCHAR tmp[3] = {0, ':', 0}; int j = nDriveNo + (WCHAR) 'A'; tmp[0] = (short) j; - wcscpy (dosname, (LPWSTR) DOS_MOUNT_PREFIX); - wcscat (dosname, tmp); + RtlStringCbCopyW (dosname, cbDosName, (LPWSTR) DOS_MOUNT_PREFIX); + RtlStringCbCatW (dosname, cbDosName, tmp); } #ifdef _DEBUG @@ -2292,7 +2292,7 @@ NTSTATUS TCOpenFsVolume (PEXTENSION Extension, PHANDLE volumeHandle, PFILE_OBJEC IO_STATUS_BLOCK ioStatus; WCHAR volumeName[TC_MAX_PATH]; - TCGetNTNameFromNumber (volumeName, Extension->nDosDriveNo); + TCGetNTNameFromNumber (volumeName, sizeof(volumeName),Extension->nDosDriveNo); RtlInitUnicodeString (&fullFileName, volumeName); InitializeObjectAttributes (&objectAttributes, &fullFileName, OBJ_CASE_INSENSITIVE | OBJ_KERNEL_HANDLE, NULL, NULL); @@ -2421,8 +2421,8 @@ NTSTATUS CreateDriveLink (int nDosDriveNo) UNICODE_STRING deviceName, symLink; NTSTATUS ntStatus; - TCGetNTNameFromNumber (dev, nDosDriveNo); - TCGetDosNameFromNumber (link, nDosDriveNo); + TCGetNTNameFromNumber (dev, sizeof(dev),nDosDriveNo); + TCGetDosNameFromNumber (link, sizeof(link),nDosDriveNo); RtlInitUnicodeString (&deviceName, dev); RtlInitUnicodeString (&symLink, link); @@ -2439,7 +2439,7 @@ NTSTATUS RemoveDriveLink (int nDosDriveNo) UNICODE_STRING symLink; NTSTATUS ntStatus; - TCGetDosNameFromNumber (link, nDosDriveNo); + TCGetDosNameFromNumber (link, sizeof(link),nDosDriveNo); RtlInitUnicodeString (&symLink, link); ntStatus = IoDeleteSymbolicLink (&symLink); @@ -2457,15 +2457,15 @@ NTSTATUS MountManagerMount (MOUNT_STRUCT *mount) PMOUNTMGR_CREATE_POINT_INPUT point = (PMOUNTMGR_CREATE_POINT_INPUT) buf; UNICODE_STRING symName, devName; - TCGetNTNameFromNumber (arrVolume, mount->nDosDriveNo); + TCGetNTNameFromNumber (arrVolume, sizeof(arrVolume),mount->nDosDriveNo); in->DeviceNameLength = (USHORT) wcslen (arrVolume) * 2; - wcscpy(in->DeviceName, arrVolume); + RtlStringCbCopyW(in->DeviceName, sizeof(buf) - sizeof(in->DeviceNameLength),arrVolume); ntStatus = TCDeviceIoControl (MOUNTMGR_DEVICE_NAME, IOCTL_MOUNTMGR_VOLUME_ARRIVAL_NOTIFICATION, in, (ULONG) (sizeof (in->DeviceNameLength) + wcslen (arrVolume) * 2), 0, 0); memset (buf, 0, sizeof buf); - TCGetDosNameFromNumber ((PWSTR) &point[1], mount->nDosDriveNo); + TCGetDosNameFromNumber ((PWSTR) &point[1], sizeof(buf) - sizeof(MOUNTMGR_CREATE_POINT_INPUT),mount->nDosDriveNo); point->SymbolicLinkNameOffset = sizeof (MOUNTMGR_CREATE_POINT_INPUT); point->SymbolicLinkNameLength = (USHORT) wcslen ((PWSTR) &point[1]) * 2; @@ -2473,7 +2473,7 @@ NTSTATUS MountManagerMount (MOUNT_STRUCT *mount) RtlInitUnicodeString(&symName, (PWSTR) (buf + point->SymbolicLinkNameOffset)); point->DeviceNameOffset = point->SymbolicLinkNameOffset + point->SymbolicLinkNameLength; - TCGetNTNameFromNumber ((PWSTR) (buf + point->DeviceNameOffset), mount->nDosDriveNo); + TCGetNTNameFromNumber ((PWSTR) (buf + point->DeviceNameOffset), sizeof(buf) - point->DeviceNameOffset,mount->nDosDriveNo); point->DeviceNameLength = (USHORT) wcslen ((PWSTR) (buf + point->DeviceNameOffset)) * 2; RtlInitUnicodeString(&devName, (PWSTR) (buf + point->DeviceNameOffset)); @@ -2493,7 +2493,7 @@ NTSTATUS MountManagerUnmount (int nDosDriveNo) memset (buf, 0, sizeof buf); - TCGetDosNameFromNumber ((PWSTR) &in[1], nDosDriveNo); + TCGetDosNameFromNumber ((PWSTR) &in[1], sizeof(buf) - sizeof(MOUNTMGR_MOUNT_POINT),nDosDriveNo); // Only symbolic link can be deleted with IOCTL_MOUNTMGR_DELETE_POINTS. If any other entry is specified, the mount manager will ignore subsequent IOCTL_MOUNTMGR_VOLUME_ARRIVAL_NOTIFICATION for the same volume ID. in->SymbolicLinkNameOffset = sizeof (MOUNTMGR_MOUNT_POINT); @@ -2885,7 +2885,7 @@ BOOL IsDriveLetterAvailable (int nDosDriveNo) WCHAR link[128]; HANDLE handle; - TCGetDosNameFromNumber (link, nDosDriveNo); + TCGetDosNameFromNumber (link, sizeof(link),nDosDriveNo); RtlInitUnicodeString (&objectName, link); InitializeObjectAttributes (&objectAttributes, &objectName, OBJ_KERNEL_HANDLE | OBJ_CASE_INSENSITIVE, NULL, NULL); diff --git a/src/Driver/Ntdriver.h b/src/Driver/Ntdriver.h index 66903b6c..5fc92843 100644 --- a/src/Driver/Ntdriver.h +++ b/src/Driver/Ntdriver.h @@ -125,8 +125,8 @@ void TCStopThread (PKTHREAD kThread, PKEVENT wakeUpEvent); void TCStopVolumeThread (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension); VOID VolumeThreadProc (PVOID Context); void TCSleep (int milliSeconds); -void TCGetNTNameFromNumber (LPWSTR ntname, int nDriveNo); -void TCGetDosNameFromNumber (LPWSTR dosname, int nDriveNo); +void TCGetNTNameFromNumber (LPWSTR ntname, int cbNtName, int nDriveNo); +void TCGetDosNameFromNumber (LPWSTR dosname, int cbDosName, int nDriveNo); LPWSTR TCTranslateCode (ULONG ulCode); void TCDeleteDeviceObject (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension); VOID TCUnloadDriver (PDRIVER_OBJECT DriverObject); diff --git a/src/Driver/Ntvol.c b/src/Driver/Ntvol.c index caaf9428..29ccd543 100644 --- a/src/Driver/Ntvol.c +++ b/src/Driver/Ntvol.c @@ -30,6 +30,8 @@ #pragma warning( disable : 4127 ) +#include + volatile BOOL ProbingHostDeviceForWrite = FALSE; @@ -380,8 +382,8 @@ NTSTATUS TCOpenVolume (PDEVICE_OBJECT DeviceObject, OBJECT_ATTRIBUTES oaParentFileAttributes; LARGE_INTEGER parentKeyDataOffset; - _snwprintf (parentDrivePath, - sizeof (parentDrivePath) / sizeof (WCHAR) - 1, + RtlStringCbPrintfW (parentDrivePath, + sizeof (parentDrivePath), WIDE ("\\Device\\Harddisk%d\\Partition0"), mount->nPartitionInInactiveSysEncScopeDriveNo); @@ -478,6 +480,14 @@ NTSTATUS TCOpenVolume (PDEVICE_OBJECT DeviceObject, { /* Volume header successfully decrypted */ + if (!Extension->cryptoInfo) + { + /* should never happen */ + mount->nReturnCode = ERR_OUTOFMEMORY; + ntStatus = STATUS_SUCCESS; + goto error; + } + Dump ("Volume header decrypted\n"); Dump ("Required program version = %x\n", (int) Extension->cryptoInfo->RequiredProgramVersion); Dump ("Legacy volume = %d\n", (int) Extension->cryptoInfo->LegacyVolume); @@ -645,14 +655,14 @@ NTSTATUS TCOpenVolume (PDEVICE_OBJECT DeviceObject, if (wcsstr (pwszMountVolume, WIDE ("\\??\\UNC\\")) == pwszMountVolume) { /* UNC path */ - _snwprintf (Extension->wszVolume, - sizeof (Extension->wszVolume) / sizeof (WCHAR) - 1, + RtlStringCbPrintfW (Extension->wszVolume, + sizeof (Extension->wszVolume), WIDE ("\\??\\\\%s"), pwszMountVolume + 7); } else { - wcsncpy (Extension->wszVolume, pwszMountVolume, sizeof (Extension->wszVolume) / sizeof (WCHAR) - 1); + RtlStringCbCopyW (Extension->wszVolume, sizeof(Extension->wszVolume),pwszMountVolume); } } -- cgit v1.2.3