Static Code Analysis: in Windows Driver, avoid using uninitialized stack memory as random and use proper random value for wipe operation. Solve potential double-free issue.

This commit is contained in:
Mounir IDRASSI 2015-02-08 23:41:37 +01:00
parent 28a9eaf0e3
commit 516da2229d
3 changed files with 48 additions and 5 deletions

View File

@ -1176,6 +1176,36 @@ static VOID SetupThreadProc (PVOID threadArg)
KIRQL irql; KIRQL irql;
NTSTATUS status; NTSTATUS status;
// generate real random values for wipeRandChars and
// wipeRandCharsUpdate instead of relying on uninitialized stack memory
LARGE_INTEGER iSeed;
KeQuerySystemTime( &iSeed );
if (KeGetCurrentIrql() < DISPATCH_LEVEL)
{
ULONG ulRandom;
ulRandom = RtlRandomEx( &iSeed.LowPart );
memcpy (wipeRandChars, &ulRandom, TC_WIPE_RAND_CHAR_COUNT);
ulRandom = RtlRandomEx( &ulRandom );
memcpy (wipeRandCharsUpdate, &ulRandom, TC_WIPE_RAND_CHAR_COUNT);
burn (&ulRandom, sizeof(ulRandom));
}
else
{
byte digest[SHA512_DIGESTSIZE];
sha512_ctx tctx;
sha512_begin (&tctx);
sha512_hash ((unsigned char *) &(iSeed.QuadPart), sizeof(iSeed.QuadPart), &tctx);
sha512_end (digest, &tctx);
memcpy (wipeRandChars, digest, TC_WIPE_RAND_CHAR_COUNT);
memcpy (wipeRandCharsUpdate, &digest[SHA512_DIGESTSIZE - TC_WIPE_RAND_CHAR_COUNT], TC_WIPE_RAND_CHAR_COUNT);
burn (digest, SHA512_DIGESTSIZE);
burn (&tctx, sizeof (tctx));
}
burn (&iSeed, sizeof(iSeed));
SetupResult = STATUS_UNSUCCESSFUL; SetupResult = STATUS_UNSUCCESSFUL;
// Make sure volume header can be updated // Make sure volume header can be updated
@ -1475,9 +1505,18 @@ static VOID SetupThreadProc (PVOID threadArg)
ret: ret:
if (buffer) if (buffer)
{
burn (buffer, TC_ENCRYPTION_SETUP_IO_BLOCK_SIZE);
TCfree (buffer); TCfree (buffer);
}
if (wipeBuffer) if (wipeBuffer)
{
burn (wipeBuffer, TC_ENCRYPTION_SETUP_IO_BLOCK_SIZE);
TCfree (wipeBuffer); TCfree (wipeBuffer);
}
burn (wipeRandChars, TC_WIPE_RAND_CHAR_COUNT);
burn (wipeRandCharsUpdate, TC_WIPE_RAND_CHAR_COUNT);
SetupInProgress = FALSE; SetupInProgress = FALSE;
PsTerminateSystemThread (SetupResult); PsTerminateSystemThread (SetupResult);

View File

@ -1709,7 +1709,7 @@ void TCStopVolumeThread (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension)
{ {
NTSTATUS ntStatus; NTSTATUS ntStatus;
if (DeviceObject); /* Remove compiler warning */ UNREFERENCED_PARAMETER (DeviceObject); /* Remove compiler warning */
Dump ("Signalling thread to quit...\n"); Dump ("Signalling thread to quit...\n");

View File

@ -726,7 +726,7 @@ NTSTATUS TCOpenVolume (PDEVICE_OBJECT DeviceObject,
void TCCloseVolume (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension) void TCCloseVolume (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension)
{ {
if (DeviceObject); /* Remove compiler warning */ UNREFERENCED_PARAMETER (DeviceObject); /* Remove compiler warning */
if (Extension->hDeviceFile != NULL) if (Extension->hDeviceFile != NULL)
{ {
@ -738,7 +738,11 @@ void TCCloseVolume (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension)
ZwClose (Extension->hDeviceFile); ZwClose (Extension->hDeviceFile);
} }
ObDereferenceObject (Extension->pfoDeviceFile); ObDereferenceObject (Extension->pfoDeviceFile);
crypto_close (Extension->cryptoInfo); if (Extension->cryptoInfo)
{
crypto_close (Extension->cryptoInfo);
Extension->cryptoInfo = NULL;
}
} }
@ -752,7 +756,7 @@ NTSTATUS TCSendHostDeviceIoControlRequest (PDEVICE_OBJECT DeviceObject,
NTSTATUS ntStatus; NTSTATUS ntStatus;
PIRP Irp; PIRP Irp;
if (DeviceObject); /* Remove compiler warning */ UNREFERENCED_PARAMETER(DeviceObject); /* Remove compiler warning */
KeClearEvent (&Extension->keVolumeEvent); KeClearEvent (&Extension->keVolumeEvent);
@ -791,7 +795,7 @@ NTSTATUS COMPLETE_IRP (PDEVICE_OBJECT DeviceObject,
Irp->IoStatus.Status = IrpStatus; Irp->IoStatus.Status = IrpStatus;
Irp->IoStatus.Information = IrpInformation; Irp->IoStatus.Information = IrpInformation;
if (DeviceObject); /* Remove compiler warning */ UNREFERENCED_PARAMETER (DeviceObject); /* Remove compiler warning */
#if EXTRA_INFO #if EXTRA_INFO
if (!NT_SUCCESS (IrpStatus)) if (!NT_SUCCESS (IrpStatus))