Windows: Add various checks to address Coverity reported issues.

author: Mounir IDRASSI <mounir.idrassi@idrix.fr> 2022-03-26 20:03:19 +0100
committer: Mounir IDRASSI <mounir.idrassi@idrix.fr> 2022-03-26 21:15:11 +0100
commit: 762065917f3ac47c3bdcacdb608d35b36dfb3973 (patch)
tree: 7863397c35f5e560c28150879307acec6c18b3d2
parent: a0809fe85c2f1bf130c26ff77aea7dac19b6c05f (diff)
download: VeraCrypt-762065917f3ac47c3bdcacdb608d35b36dfb3973.tar.gz
VeraCrypt-762065917f3ac47c3bdcacdb608d35b36dfb3973.zip
11 files changed, 105 insertions, 28 deletions
diff --git a/src/Common/Dlgcode.c b/src/Common/Dlgcode.c
index 9d5c0d06..7b3d2d45 100644
--- a/src/Common/Dlgcode.c
+++ b/src/Common/Dlgcode.c
@@ -610,8 +610,9 @@ char *LoadFile (const wchar_t *fileName, DWORD *size)
 {
 	char *buf;
 	DWORD fileSize = INVALID_FILE_SIZE;
 	HANDLE h = CreateFile (fileName, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL);
+	*size = 0;
 	if (h == INVALID_HANDLE_VALUE)
 		return NULL;
 
 	if ((fileSize = GetFileSize (h, NULL)) == INVALID_FILE_SIZE)
@@ -619,22 +620,25 @@ char *LoadFile (const wchar_t *fileName, DWORD *size)
 		CloseHandle (h);
 		return NULL;
 	}
 
-	*size = fileSize;
-	buf = (char *) calloc (*size + 1, 1);
+	buf = (char *) calloc (fileSize + 1, 1);
 
 	if (buf == NULL)
 	{
 		CloseHandle (h);
 		return NULL;
 	}
 
-	if (!ReadFile (h, buf, *size, size, NULL))
+	if (!ReadFile (h, buf, fileSize, size, NULL))
 	{
 		free (buf);
 		buf = NULL;
 	}
+	else
+	{
+		buf[*size] = 0; //make coverity happy eventhough buf is guaranteed to be null terminated because of fileSize+1 in calloc call
+	}
 
 	CloseHandle (h);
 	return buf;
 }
diff --git a/src/Common/Language.c b/src/Common/Language.c
index 844f4dad..278b7dd1 100644
--- a/src/Common/Language.c
+++ b/src/Common/Language.c
@@ -610,9 +610,10 @@ char *GetPreferredLangId ()
 
 
 void SetPreferredLangId (char *langId)
 {
-	StringCbCopyA (PreferredLangId, sizeof(PreferredLangId), langId);
+	if (strlen(langId) < sizeof(PreferredLangId))
+		StringCbCopyA (PreferredLangId, sizeof(PreferredLangId), langId);
 }
 
 
 char *GetActiveLangPackVersion ()
diff --git a/src/Common/Tests.c b/src/Common/Tests.c
index 0fcd93ce..4f53d4ed 100644
--- a/src/Common/Tests.c
+++ b/src/Common/Tests.c
@@ -1518,14 +1518,22 @@ BOOL test_hmac_sha256 ()
 
 	for (i = 0; i < sizeof (hmac_sha256_test_data) / sizeof(char *); i++)
 	{
 		char digest[1024]; /* large enough to hold digets and test vector inputs */
-		memcpy (digest, hmac_sha256_test_data[i], strlen (hmac_sha256_test_data[i]));
-		hmac_sha256 (hmac_sha256_test_keys[i], (int) strlen (hmac_sha256_test_keys[i]), digest, (int) strlen (hmac_sha256_test_data[i]));
-		if (memcmp (digest, hmac_sha256_test_vectors[i], SHA256_DIGESTSIZE) != 0)
-			return FALSE;
+		size_t dataLen = strlen (hmac_sha256_test_data[i]);
+		if (dataLen <= sizeof(digest))
+		{
+			memcpy (digest, hmac_sha256_test_data[i], dataLen);
+			hmac_sha256 (hmac_sha256_test_keys[i], (int) strlen (hmac_sha256_test_keys[i]), digest, (int) dataLen);
+			if (memcmp (digest, hmac_sha256_test_vectors[i], SHA256_DIGESTSIZE) != 0)
+				return FALSE;
+			else
+				nTestsPerformed++;
+		}
 		else
-			nTestsPerformed++;
+		{
+			return FALSE;
+		}
 	}
 
 	return (nTestsPerformed == 6);
 }
@@ -1537,14 +1545,22 @@ BOOL test_hmac_sha512 ()
 
 	for (i = 0; i < sizeof (hmac_sha512_test_data) / sizeof(char *); i++)
 	{
 		char digest[1024]; /* large enough to hold digets and test vector inputs */
-		memcpy (digest, hmac_sha512_test_data[i], (int) strlen (hmac_sha512_test_data[i]));
-		hmac_sha512 (hmac_sha512_test_keys[i], (int) strlen (hmac_sha512_test_keys[i]), digest, (int) strlen (hmac_sha512_test_data[i]));
-		if (memcmp (digest, hmac_sha512_test_vectors[i], SHA512_DIGESTSIZE) != 0)
-			return FALSE;
+		size_t dataLen = strlen (hmac_sha512_test_data[i]);
+		if (dataLen <= sizeof(digest))
+		{
+			memcpy (digest, hmac_sha512_test_data[i], dataLen );
+			hmac_sha512 (hmac_sha512_test_keys[i], (int) strlen (hmac_sha512_test_keys[i]), digest, (int) dataLen);
+			if (memcmp (digest, hmac_sha512_test_vectors[i], SHA512_DIGESTSIZE) != 0)
+				return FALSE;
+			else
+				nTestsPerformed++;
+		}
 		else
-			nTestsPerformed++;
+		{
+			return FALSE;
+		}
 	}
 
 	return (nTestsPerformed == 6);
 }
@@ -1556,14 +1572,22 @@ BOOL test_hmac_blake2s ()
 
 	for (i = 0; i < sizeof (hmac_blake2s_test_data) / sizeof(char *); i++)
 	{
 		char digest[1024]; /* large enough to hold digets and test vector inputs */
-		memcpy (digest, hmac_blake2s_test_data[i], strlen (hmac_blake2s_test_data[i]));
-		hmac_blake2s (hmac_blake2s_test_keys[i], (int) strlen (hmac_blake2s_test_keys[i]), digest, (int) strlen (hmac_blake2s_test_data[i]));
-		if (memcmp (digest, hmac_blake2s_test_vectors[i], BLAKE2S_DIGESTSIZE) != 0)
-			return FALSE;
+		size_t dataLen = strlen (hmac_blake2s_test_data[i]);
+		if (dataLen <= sizeof(digest))
+		{
+			memcpy (digest, hmac_blake2s_test_data[i], dataLen);
+			hmac_blake2s (hmac_blake2s_test_keys[i], (int) strlen (hmac_blake2s_test_keys[i]), digest, (int) dataLen);
+			if (memcmp (digest, hmac_blake2s_test_vectors[i], BLAKE2S_DIGESTSIZE) != 0)
+				return FALSE;
+			else
+				nTestsPerformed++;
+		}
 		else
-			nTestsPerformed++;
+		{
+			return FALSE;
+		}
 	}
 
 	return (nTestsPerformed == 6);
 }
diff --git a/src/Crypto/cpu.c b/src/Crypto/cpu.c
index 27d62e43..c3a769c8 100644
--- a/src/Crypto/cpu.c
+++ b/src/Crypto/cpu.c
@@ -279,9 +279,9 @@ static int Detect_MS_HyperV_AES ()
 	int hasAesNI = 0;
 	// when Hyper-V is enabled on older versions of Windows Server (i.e. 2008 R2), the AES-NI capability 
 	// gets masked out for all applications, even running on the host.
 	// We try to detect Hyper-V virtual CPU and perform a dummy AES-NI operation to check its real presence
-	uint32 cpuid[4];
+	uint32 cpuid[4] = {0};
 	char HvProductName[13];
 
 	CpuId(0x40000000, cpuid);
 	memcpy (HvProductName, &cpuid[1], 12);
@@ -347,9 +347,9 @@ void DetectX86Features()
 	if ((cpuid1[3] & (1 << 25)) != 0)
 		g_hasISSE = 1;
 	else
 	{
-		uint32 cpuid2[4];
+		uint32 cpuid2[4] = {0};
 		CpuId(0x080000000, cpuid2);
 		if (cpuid2[0] >= 0x080000001)
 		{
 			CpuId(0x080000001, cpuid2);
diff --git a/src/ExpandVolume/ExpandVolume.c b/src/ExpandVolume/ExpandVolume.c
index 84f6cfe8..f62d93ae 100644
--- a/src/ExpandVolume/ExpandVolume.c
+++ b/src/ExpandVolume/ExpandVolume.c
@@ -441,8 +441,14 @@ int ExtendFileSystem (HWND hwndDlg , wchar_t *lpszVolume, Password *pVolumePassw
 		nStatus = ERR_OS_ERROR;
 		goto error;
 	}
 
+	if ((BytesPerSector == 0) || (BytesPerSector > (DWORD)INT_MAX))
+	{
+		nStatus = ERR_SECTOR_SIZE_INCOMPATIBLE;
+		goto error;
+	}
+
 	DebugAddProgressDlgStatus (hwndDlg, L"Extending file system ...\r\n");
 
 	// extend volume
 	nStatus = FsctlExtendVolume(szVolumeGUID, newDataAreaSize/BytesPerSector );
diff --git a/src/Format/InPlace.c b/src/Format/InPlace.c
index 4a16fd4f..f6166dab 100644
--- a/src/Format/InPlace.c
+++ b/src/Format/InPlace.c
@@ -88,8 +88,10 @@ static __int64 NewFileSysSizeAfterShrink (HANDLE dev, const wchar_t *devicePath,
 		return -1;
 	}
 
 	if (	(ntfsVolData.NumberSectors.QuadPart <= 0)
+		||	(ntfsVolData.BytesPerSector == 0)
+		||	(ntfsVolData.BytesPerSector >= (DWORD) UINT_MAX)
 		||	(ntfsVolData.NumberSectors.QuadPart > (INT64_MAX / (__int64) ntfsVolData.BytesPerSector)) // overflow test
 		)
 	{
 		SetLastError (ERROR_INTERNAL_ERROR);
diff --git a/src/Format/Tcformat.c b/src/Format/Tcformat.c
index 477306ea..efd95caf 100644
--- a/src/Format/Tcformat.c
+++ b/src/Format/Tcformat.c
@@ -9755,13 +9755,20 @@ int AnalyzeHiddenVolumeHost (HWND hwndDlg, int *driveNo, __int64 hiddenVolHostSi
 		// Get the map of the clusters that are free and in use on the outer volume.
 		// The map will be scanned to determine the size of the uninterrupted block of free
 		// space (provided there is any) whose end is aligned with the end of the volume.
 		// The value will then be used to determine the maximum possible size of the hidden volume.
-
-		return ScanVolClusterBitmap (hwndDlg,
-			driveNo,
-			hiddenVolHostSize / *realClusterSize,
-			pnbrFreeClusters);
+		if (*realClusterSize > 0)
+		{
+			return ScanVolClusterBitmap (hwndDlg,
+				driveNo,
+				hiddenVolHostSize / *realClusterSize,
+				pnbrFreeClusters);
+		}
+		else
+		{
+			// should never happen
+			return -1;
+		}
 	}
 	else if (!wcsncmp (szFileSystemNameBuffer, L"NTFS", 4) || !_wcsnicmp (szFileSystemNameBuffer, L"exFAT", 5))
 	{
 		// NTFS
diff --git a/src/Mount/Mount.c b/src/Mount/Mount.c
index a0370861..a5798afc 100644
--- a/src/Mount/Mount.c
+++ b/src/Mount/Mount.c
@@ -6582,8 +6582,16 @@ static void ShowSystemEncryptionStatus (HWND hwndDlg)
 
 	if (GetAsyncKeyState (VK_SHIFT) < 0 && GetAsyncKeyState (VK_CONTROL) < 0)
 	{
 		// Ctrl+Shift held (for debugging purposes)
+		int64 encryptedRatio = 0;
+		if (BootEncStatus.DriveEncrypted 
+			&& (BootEncStatus.ConfiguredEncryptedAreaStart >= 0) 
+			&& (BootEncStatus.ConfiguredEncryptedAreaEnd >= BootEncStatus.ConfiguredEncryptedAreaStart)
+			)
+		{
+			encryptedRatio = (BootEncStatus.EncryptedAreaEnd + 1 - BootEncStatus.EncryptedAreaStart) * 100I64 / (BootEncStatus.ConfiguredEncryptedAreaEnd + 1 - BootEncStatus.ConfiguredEncryptedAreaStart);
+		}
 
 		DebugMsgBox ("Debugging information for system encryption:\n\nDeviceFilterActive: %d\nBootLoaderVersion: %x\nSetupInProgress: %d\nSetupMode: %d\nVolumeHeaderPresent: %d\nDriveMounted: %d\nDriveEncrypted: %d\n"
 			"HiddenSystem: %d\nHiddenSystemPartitionStart: %I64d\n"
 			"ConfiguredEncryptedAreaStart: %I64d\nConfiguredEncryptedAreaEnd: %I64d\nEncryptedAreaStart: %I64d\nEncryptedAreaEnd: %I64d\nEncrypted: %I64d%%",
@@ -6599,9 +6607,9 @@ static void ShowSystemEncryptionStatus (HWND hwndDlg)
 			BootEncStatus.ConfiguredEncryptedAreaStart,
 			BootEncStatus.ConfiguredEncryptedAreaEnd,
 			BootEncStatus.EncryptedAreaStart,
 			BootEncStatus.EncryptedAreaEnd,
-			!BootEncStatus.DriveEncrypted ? 0 : (BootEncStatus.EncryptedAreaEnd + 1 - BootEncStatus.EncryptedAreaStart) * 100I64 / (BootEncStatus.ConfiguredEncryptedAreaEnd + 1 - BootEncStatus.ConfiguredEncryptedAreaStart));
+			encryptedRatio);
 	}
 
 	if (!BootEncStatus.DriveEncrypted && !BootEncStatus.DriveMounted)
 	{
diff --git a/src/Setup/Dir.c b/src/Setup/Dir.c
index 2d4feecd..3275567f 100644
--- a/src/Setup/Dir.c
+++ b/src/Setup/Dir.c
@@ -30,8 +30,14 @@ mkfulldir (wchar_t *oriPath, BOOL bCheckonly)
 	struct _stat st;
 	wchar_t *uniq_file;
 	wchar_t path [TC_MAX_PATH];
 
+	if (wcslen(oriPath) >= TC_MAX_PATH)
+	{
+		// directory name will be truncated so return failure to avoid unexepected behavior
+		return -1;
+	}
+
 	StringCbCopyW (path, TC_MAX_PATH, oriPath);
 
 	if (wcslen (path) == 3 && path[1] == L':')
 		goto is_root;	/* keep final slash in root if present */
@@ -65,8 +71,14 @@ mkfulldir_internal (wchar_t *path)
 	struct _stat st;
 	static wchar_t tokpath[_MAX_PATH];
 	static wchar_t trail[_MAX_PATH];
 
+	if (wcslen(path) >= _MAX_PATH)
+	{
+		// directory name will be truncated so return failure to avoid unexepected behavior
+		return -1;
+	}
+
 	StringCbCopyW (tokpath, _MAX_PATH, path);
 	trail[0] = L'\0';
 
 	token = wcstok (tokpath, L"\\/");
diff --git a/src/Setup/Setup.c b/src/Setup/Setup.c
index 9433bd40..43c951f5 100644
--- a/src/Setup/Setup.c
+++ b/src/Setup/Setup.c
@@ -818,9 +818,10 @@ BOOL DoFilesInstall (HWND hwndDlg, wchar_t *szDestDir)
 		{
 			if (Is64BitOs ())
 				driver64 = TRUE;
 
-			GetSystemDirectory (szDir, ARRAYSIZE (szDir));
+			if (!GetSystemDirectory (szDir, ARRAYSIZE (szDir)))
+				StringCbCopyW(szDir, sizeof(szDir), L"C:\\Windows\\System32");
 
 			x = wcslen (szDir);
 			if (szDir[x - 1] != L'\\')
 				StringCbCatW (szDir, sizeof(szDir), L"\\");
diff --git a/src/SetupDLL/Dir.c b/src/SetupDLL/Dir.c
index 2d4feecd..3275567f 100644
--- a/src/SetupDLL/Dir.c
+++ b/src/SetupDLL/Dir.c
@@ -30,8 +30,14 @@ mkfulldir (wchar_t *oriPath, BOOL bCheckonly)
 	struct _stat st;
 	wchar_t *uniq_file;
 	wchar_t path [TC_MAX_PATH];
 
+	if (wcslen(oriPath) >= TC_MAX_PATH)
+	{
+		// directory name will be truncated so return failure to avoid unexepected behavior
+		return -1;
+	}
+
 	StringCbCopyW (path, TC_MAX_PATH, oriPath);
 
 	if (wcslen (path) == 3 && path[1] == L':')
 		goto is_root;	/* keep final slash in root if present */
@@ -65,8 +71,14 @@ mkfulldir_internal (wchar_t *path)
 	struct _stat st;
 	static wchar_t tokpath[_MAX_PATH];
 	static wchar_t trail[_MAX_PATH];
 
+	if (wcslen(path) >= _MAX_PATH)
+	{
+		// directory name will be truncated so return failure to avoid unexepected behavior
+		return -1;
+	}
+
 	StringCbCopyW (tokpath, _MAX_PATH, path);
 	trail[0] = L'\0';
 
 	token = wcstok (tokpath, L"\\/");
author	Mounir IDRASSI <mounir.idrassi@idrix.fr>	2022-03-26 20:03:19 +0100
committer	Mounir IDRASSI <mounir.idrassi@idrix.fr>	2022-03-26 21:15:11 +0100
commit	762065917f3ac47c3bdcacdb608d35b36dfb3973 (patch)
tree	7863397c35f5e560c28150879307acec6c18b3d2
parent	a0809fe85c2f1bf130c26ff77aea7dac19b6c05f (diff)
download	VeraCrypt-762065917f3ac47c3bdcacdb608d35b36dfb3973.tar.gz VeraCrypt-762065917f3ac47c3bdcacdb608d35b36dfb3973.zip