From ce2aa639f448e4585e953fd14292dc0a9c5c4d86 Mon Sep 17 00:00:00 2001 From: Mounir IDRASSI Date: Sun, 11 Jun 2017 17:26:42 +0200 Subject: Windows: various fixes following Coverity analysis. --- src/Common/BootEncryption.cpp | 5 +- src/Common/Dlgcode.c | 158 +++++++++++++++++++++++++++++------------- src/Common/Dlgcode.h | 1 + src/Common/Format.c | 5 +- 4 files changed, 116 insertions(+), 53 deletions(-) (limited to 'src/Common') diff --git a/src/Common/BootEncryption.cpp b/src/Common/BootEncryption.cpp index 60985751..4e505eca 100644 --- a/src/Common/BootEncryption.cpp +++ b/src/Common/BootEncryption.cpp @@ -2137,6 +2137,7 @@ namespace VeraCrypt ZeroMemory (&sdn, sizeof (sdn)); ZeroMemory (&partInfo, sizeof (partInfo)); m_bMounted = false; + bBootVolumePathSelected = false; } void EfiBoot::SelectBootVolumeESP() { @@ -2161,14 +2162,14 @@ namespace VeraCrypt PUNICODE_STRING pStr = (PUNICODE_STRING) tempBuf; memcpy (BootVolumePath, pStr->Buffer, min (pStr->Length, (sizeof (BootVolumePath) - 2))); - bBootVolumePathSelected = TRUE; + bBootVolumePathSelected = true; } void EfiBoot::SelectBootVolume(WCHAR* bootVolumePath) { wstring str; str = bootVolumePath; memcpy (BootVolumePath, &str[0], min (str.length() * 2, (sizeof (BootVolumePath) - 2))); - bBootVolumePathSelected = TRUE; + bBootVolumePathSelected = true; } void EfiBoot::MountBootPartition(WCHAR letter) { diff --git a/src/Common/Dlgcode.c b/src/Common/Dlgcode.c index af3bec0b..31d05944 100644 --- a/src/Common/Dlgcode.c +++ b/src/Common/Dlgcode.c @@ -547,6 +547,20 @@ size_t TrimWhiteSpace(wchar_t *str) return out_size; } +BOOL IsNullTerminateString (const wchar_t* str, size_t cbSize) +{ + if (str && cbSize) + { + for (size_t i = 0; i < cbSize; i++) + { + if (str[i] == 0) + return TRUE; + } + } + + return FALSE; +} + // check the validity of a file name BOOL IsValidFileName(const wchar_t* str) { @@ -8212,9 +8226,14 @@ BOOL IsMountedVolumeID (BYTE volumeID[VOLUME_ID_SIZE]) int i; memset (&mlist, 0, sizeof (mlist)); - DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, - sizeof (mlist), &mlist, sizeof (mlist), &dwResult, - NULL); + if ( !DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, + sizeof (mlist), &mlist, sizeof (mlist), &dwResult, + NULL) + || (mlist.ulMountedDrives >= (1 << 26)) + ) + { + return FALSE; + } if (mlist.ulMountedDrives) { @@ -8256,16 +8275,26 @@ BOOL IsMountedVolume (const wchar_t *volname) StringCbCopyW (volume, sizeof (volume), resolvedPath.c_str()); memset (&mlist, 0, sizeof (mlist)); - DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, - sizeof (mlist), &mlist, sizeof (mlist), &dwResult, - NULL); + if ( !DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, + sizeof (mlist), &mlist, sizeof (mlist), &dwResult, + NULL) + || (mlist.ulMountedDrives >= (1 << 26)) + ) + { + return FALSE; + } if (mlist.ulMountedDrives) { for (i=0 ; i<26; i++) { - if ((mlist.ulMountedDrives & (1 << i)) && (0 == _wcsicmp ((wchar_t *) mlist.wszVolume[i], volume))) + if ((mlist.ulMountedDrives & (1 << i)) + && IsNullTerminateString (mlist.wszVolume[i], TC_MAX_PATH) + && (0 == _wcsicmp ((wchar_t *) mlist.wszVolume[i], volume)) + ) + { return TRUE; + } } } } @@ -8294,16 +8323,26 @@ int GetMountedVolumeDriveNo (wchar_t *volname) StringCbCopyW (volume, sizeof (volume), resolvedPath.c_str()); memset (&mlist, 0, sizeof (mlist)); - DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, - sizeof (mlist), &mlist, sizeof (mlist), &dwResult, - NULL); + if ( !DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, + sizeof (mlist), &mlist, sizeof (mlist), &dwResult, + NULL) + || (mlist.ulMountedDrives >= (1 << 26)) + ) + { + return -1; + } if (mlist.ulMountedDrives) { for (i=0 ; i<26; i++) { - if ((mlist.ulMountedDrives & (1 << i)) && (0 == _wcsicmp ((wchar_t *) mlist.wszVolume[i], (WCHAR *)volume))) + if ((mlist.ulMountedDrives & (1 << i)) + && IsNullTerminateString (mlist.wszVolume[i], TC_MAX_PATH) + && (0 == _wcsicmp ((wchar_t *) mlist.wszVolume[i], (WCHAR *)volume)) + ) + { return i; + } } } @@ -8425,7 +8464,7 @@ BOOL GetDriveGeometry (const wchar_t *deviceName, PDISK_GEOMETRY_EX diskGeometry if (bResult && (dwResult == sizeof (dg)) && dg.diskGeometry.BytesPerSector) { - ZeroMemory (diskGeometry, sizeof (PDISK_GEOMETRY_EX)); + ZeroMemory (diskGeometry, sizeof (DISK_GEOMETRY_EX)); memcpy (&diskGeometry->Geometry, &dg.diskGeometry, sizeof (DISK_GEOMETRY)); diskGeometry->DiskSize.QuadPart = dg.DiskSize.QuadPart; return TRUE; @@ -9341,11 +9380,19 @@ LRESULT ListSubItemSet (HWND list, int index, int subIndex, const wchar_t *strin BOOL GetMountList (MOUNT_LIST_STRUCT *list) { DWORD dwResult; + MOUNT_LIST_STRUCT localList = {0}; - memset (list, 0, sizeof (*list)); - return DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, list, - sizeof (*list), list, sizeof (*list), &dwResult, - NULL); + if ( list && DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &localList, + sizeof (localList), &localList, sizeof (localList), &dwResult, + NULL) + && (localList.ulMountedDrives < (1 << 26)) + ) + { + memcpy (list, &localList, sizeof (MOUNT_LIST_STRUCT)); + return TRUE; + } + else + return FALSE; } @@ -11853,44 +11900,47 @@ std::vector GetHostRawDeviceList () NULL ) && ( ERROR_INSUFFICIENT_BUFFER == GetLastError())) { deviceInterfaceDetailData = ( PSP_DEVICE_INTERFACE_DETAIL_DATA ) malloc( requiredSize ); - ZeroMemory( deviceInterfaceDetailData, requiredSize ); - deviceInterfaceDetailData->cbSize = sizeof( SP_DEVICE_INTERFACE_DETAIL_DATA ); - if (SetupDiGetDeviceInterfaceDetail( diskClassDevices, - &deviceInterfaceData, - deviceInterfaceDetailData, - requiredSize, - NULL, - NULL )) + if (deviceInterfaceDetailData) { - HANDLE disk = CreateFile( deviceInterfaceDetailData->DevicePath, - 0, - FILE_SHARE_READ | FILE_SHARE_WRITE, + ZeroMemory( deviceInterfaceDetailData, requiredSize ); + deviceInterfaceDetailData->cbSize = sizeof( SP_DEVICE_INTERFACE_DETAIL_DATA ); + if (SetupDiGetDeviceInterfaceDetail( diskClassDevices, + &deviceInterfaceData, + deviceInterfaceDetailData, + requiredSize, NULL, - OPEN_EXISTING, - 0, - NULL ); - if ( INVALID_HANDLE_VALUE != disk) + NULL )) { - if (DeviceIoControl( disk, - IOCTL_STORAGE_GET_DEVICE_NUMBER, + HANDLE disk = CreateFile( deviceInterfaceDetailData->DevicePath, + 0, + FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, + OPEN_EXISTING, 0, - &diskNumber, - sizeof( STORAGE_DEVICE_NUMBER ), - &bytesReturned, - NULL )) + NULL ); + if ( INVALID_HANDLE_VALUE != disk) { - HostDevice device; - device.Path = deviceInterfaceDetailData->DevicePath; - device.SystemNumber = diskNumber.DeviceNumber; - list.push_back (device); - } + if (DeviceIoControl( disk, + IOCTL_STORAGE_GET_DEVICE_NUMBER, + NULL, + 0, + &diskNumber, + sizeof( STORAGE_DEVICE_NUMBER ), + &bytesReturned, + NULL )) + { + HostDevice device; + device.Path = deviceInterfaceDetailData->DevicePath; + device.SystemNumber = diskNumber.DeviceNumber; + list.push_back (device); + } - CloseHandle( disk ); + CloseHandle( disk ); + } } - } - free (deviceInterfaceDetailData); + free (deviceInterfaceDetailData); + } } } @@ -12107,16 +12157,26 @@ wstring FindDeviceByVolumeID (const BYTE volumeID [VOLUME_ID_SIZE]) DWORD dwResult; memset (&mlist, 0, sizeof (mlist)); - DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, - sizeof (mlist), &mlist, sizeof (mlist), &dwResult, - NULL); + if ( !DeviceIoControl (hDriver, TC_IOCTL_GET_MOUNTED_VOLUMES, &mlist, + sizeof (mlist), &mlist, sizeof (mlist), &dwResult, + NULL) + || (mlist.ulMountedDrives >= (1 << 26)) + ) + { + return L""; + } if (mlist.ulMountedDrives) { for (int i=0 ; i < 26; i++) { if ((mlist.ulMountedDrives & (1 << i)) && (0 == memcmp (mlist.volumeID[i], volumeID, VOLUME_ID_SIZE))) - return mlist.wszVolume[i]; + { + if (IsNullTerminateString (mlist.wszVolume[i], TC_MAX_PATH)) + return mlist.wszVolume[i]; + else + return L""; + } } } diff --git a/src/Common/Dlgcode.h b/src/Common/Dlgcode.h index a585218c..2f1a11fd 100644 --- a/src/Common/Dlgcode.h +++ b/src/Common/Dlgcode.h @@ -241,6 +241,7 @@ typedef struct void cleanup ( void ); void LowerCaseCopy ( wchar_t *lpszDest , const wchar_t *lpszSource ); void UpperCaseCopy ( wchar_t *lpszDest , size_t cbDest, const wchar_t *lpszSource ); +BOOL IsNullTerminateString (const wchar_t* str, size_t cbSize); void CreateFullVolumePath ( wchar_t *lpszDiskFile , size_t cbDiskFile, const wchar_t *lpszFileName , BOOL *bDevice ); int FakeDosNameForDevice ( const wchar_t *lpszDiskFile , wchar_t *lpszDosDevice , size_t cbDosDevice, wchar_t *lpszCFDevice , size_t cbCFDevice, BOOL bNameOnly ); int RemoveFakeDosName ( wchar_t *lpszDiskFile , wchar_t *lpszDosDevice ); diff --git a/src/Common/Format.c b/src/Common/Format.c index a7445a9d..cd211db7 100644 --- a/src/Common/Format.c +++ b/src/Common/Format.c @@ -163,11 +163,12 @@ int TCFormatVolume (volatile FORMAT_VOL_PARAMETERS *volParams) FormatSectorSize, FALSE); - if (nStatus != 0) + /* cryptoInfo sanity check to make Coverity happy eventhough it can't be NULL if nStatus = 0 */ + if ((nStatus != 0) || !cryptoInfo) { burn (header, sizeof (header)); VirtualUnlock (header, sizeof (header)); - return nStatus; + return nStatus? nStatus : ERR_OUTOFMEMORY; } begin_format: -- cgit v1.2.3