From 516da2229d66232f8c6ad84f5fecbdfc8c8f9f67 Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Sun, 8 Feb 2015 23:41:37 +0100 Subject: [PATCH] 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. --- src/Driver/DriveFilter.c | 39 +++++++++++++++++++++++++++++++++++++++ src/Driver/Ntdriver.c | 2 +- src/Driver/Ntvol.c | 12 ++++++++---- 3 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/Driver/DriveFilter.c b/src/Driver/DriveFilter.c index df1e2356..73a1a535 100644 --- a/src/Driver/DriveFilter.c +++ b/src/Driver/DriveFilter.c @@ -1176,6 +1176,36 @@ static VOID SetupThreadProc (PVOID threadArg) KIRQL irql; 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; // Make sure volume header can be updated @@ -1475,9 +1505,18 @@ static VOID SetupThreadProc (PVOID threadArg) ret: if (buffer) + { + burn (buffer, TC_ENCRYPTION_SETUP_IO_BLOCK_SIZE); TCfree (buffer); + } if (wipeBuffer) + { + burn (wipeBuffer, TC_ENCRYPTION_SETUP_IO_BLOCK_SIZE); TCfree (wipeBuffer); + } + + burn (wipeRandChars, TC_WIPE_RAND_CHAR_COUNT); + burn (wipeRandCharsUpdate, TC_WIPE_RAND_CHAR_COUNT); SetupInProgress = FALSE; PsTerminateSystemThread (SetupResult); diff --git a/src/Driver/Ntdriver.c b/src/Driver/Ntdriver.c index 7535ed21..86f3d9eb 100644 --- a/src/Driver/Ntdriver.c +++ b/src/Driver/Ntdriver.c @@ -1709,7 +1709,7 @@ void TCStopVolumeThread (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension) { NTSTATUS ntStatus; - if (DeviceObject); /* Remove compiler warning */ + UNREFERENCED_PARAMETER (DeviceObject); /* Remove compiler warning */ Dump ("Signalling thread to quit...\n"); diff --git a/src/Driver/Ntvol.c b/src/Driver/Ntvol.c index 32702929..7556b4cf 100644 --- a/src/Driver/Ntvol.c +++ b/src/Driver/Ntvol.c @@ -726,7 +726,7 @@ NTSTATUS TCOpenVolume (PDEVICE_OBJECT DeviceObject, void TCCloseVolume (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension) { - if (DeviceObject); /* Remove compiler warning */ + UNREFERENCED_PARAMETER (DeviceObject); /* Remove compiler warning */ if (Extension->hDeviceFile != NULL) { @@ -738,7 +738,11 @@ void TCCloseVolume (PDEVICE_OBJECT DeviceObject, PEXTENSION Extension) ZwClose (Extension->hDeviceFile); } 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; PIRP Irp; - if (DeviceObject); /* Remove compiler warning */ + UNREFERENCED_PARAMETER(DeviceObject); /* Remove compiler warning */ KeClearEvent (&Extension->keVolumeEvent); @@ -791,7 +795,7 @@ NTSTATUS COMPLETE_IRP (PDEVICE_OBJECT DeviceObject, Irp->IoStatus.Status = IrpStatus; Irp->IoStatus.Information = IrpInformation; - if (DeviceObject); /* Remove compiler warning */ + UNREFERENCED_PARAMETER (DeviceObject); /* Remove compiler warning */ #if EXTRA_INFO if (!NT_SUCCESS (IrpStatus))