Static Code Analysis: Avoid over-flaw in arithmetic operations by adding more checks. Add extra checks. Solve various issues.

src/Format/InPlace.c

@@ -21,6 +21,7 @@ IMPORTANT: Due to this issue, functions in this file must not directly interact
 #include <stdlib.h>
 #include <string.h>
 #include <string>
+#include <intsafe.h>
 
 #include "Tcdefs.h"
 #include "Platform/Finally.h"
@@ -71,6 +72,17 @@ static __int64 NewFileSysSizeAfterShrink (HANDLE dev, const char *devicePath, in
 		return -1;
 	}
 
+	if (	(ntfsVolData.NumberSectors.QuadPart <= 0)
+		||	(ntfsVolData.NumberSectors.QuadPart > (INT64_MAX / (__int64) ntfsVolData.BytesPerSector)) // overflow test
+		)
+	{
+		SetLastError (ERROR_INTERNAL_ERROR);
+		if (!silent)
+			handleWin32Error (MainDlg);
+
+		return -1;
+	}	
+
 	fileSysSize = ntfsVolData.NumberSectors.QuadPart * ntfsVolData.BytesPerSector;
 
 	desiredNbrSectors = (fileSysSize - TC_TOTAL_VOLUME_HEADERS_SIZE) / ntfsVolData.BytesPerSector;

src/Format/Tcformat.c

@@ -2537,13 +2537,12 @@ static void __cdecl volTransformThreadFunction (void *hwndDlgArg)
 			if (!bInPlaceEncNonSys)
 				SetTimer (hwndDlg, TIMER_ID_RANDVIEW, TIMER_INTERVAL_RANDVIEW, NULL);
 
-			if (volParams != NULL)
+
-			{
+			// volParams is ensured to be non NULL at this stage
-				burn ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
+			burn ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
-				VirtualUnlock ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
+			VirtualUnlock ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
-				free ((LPVOID) volParams);
+			free ((LPVOID) volParams);
-				volParams = NULL;
+			volParams = NULL;
-			}
 
 			bVolTransformThreadRunning = FALSE;
 			bVolTransformThreadCancel = FALSE;
@@ -9027,6 +9026,7 @@ int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance, char *lpszComm
 	DialogBoxParamW (hInstance, MAKEINTRESOURCEW (IDD_VOL_CREATION_WIZARD_DLG), NULL, (DLGPROC) MainDialogProc, 
 		(LPARAM)lpszCommandLine);
 
+	FinalizeApp ();
 	return 0;
 }
 

src/Mount/MainCom.cpp

@@ -255,7 +255,17 @@ extern "C" int UacBackupVolumeHeader (HWND hwndDlg, BOOL bRequireConfirmation, c
 	CoInitialize (NULL);
 
 	if (ComGetInstance (hwndDlg, &tc))
-		r = tc->BackupVolumeHeader ((LONG_PTR) hwndDlg, bRequireConfirmation, CComBSTR (lpszVolume));
+	{
+		CComBSTR volumeBstr;
+		BSTR bstr = A2WBSTR(lpszVolume);
+		if (bstr)
+		{
+			volumeBstr.Attach (bstr);
+			r = tc->BackupVolumeHeader ((LONG_PTR) hwndDlg, bRequireConfirmation, volumeBstr);
+		}
+		else
+			r = ERR_OUTOFMEMORY;
+	}
 	else
 		r = -1;
 
@@ -273,7 +283,17 @@ extern "C" int UacRestoreVolumeHeader (HWND hwndDlg, char *lpszVolume)
 	CoInitialize (NULL);
 
 	if (ComGetInstance (hwndDlg, &tc))
-		r = tc->RestoreVolumeHeader ((LONG_PTR) hwndDlg, CComBSTR (lpszVolume));
+	{
+		CComBSTR volumeBstr;
+		BSTR bstr = A2WBSTR(lpszVolume);
+		if (bstr)
+		{
+			volumeBstr.Attach (bstr);
+			r = tc->RestoreVolumeHeader ((LONG_PTR) hwndDlg, volumeBstr);
+		}
+		else
+			r = ERR_OUTOFMEMORY;
+	}
 	else
 		r = -1;
 
@@ -291,7 +311,16 @@ extern "C" int UacChangePwd (char *lpszVolume, Password *oldPassword, int old_pk
 	if (ComGetInstance (hwndDlg, &tc))
 	{
 		WaitCursor ();
-		r = tc->ChangePasswordEx2 (CComBSTR (lpszVolume), oldPassword, old_pkcs5, truecryptMode, newPassword, pkcs5, wipePassCount, (LONG_PTR) hwndDlg);
+		CComBSTR volumeBstr;
+		BSTR bstr = A2WBSTR(lpszVolume);
+		if (bstr)
+		{
+			volumeBstr.Attach (bstr);
+
+			r = tc->ChangePasswordEx2 (volumeBstr, oldPassword, old_pkcs5, truecryptMode, newPassword, pkcs5, wipePassCount, (LONG_PTR) hwndDlg);
+		}
+		else
+			r = ERR_OUTOFMEMORY;
 		NormalCursor ();
 	}
 	else

src/Mount/Mount.c

@@ -719,8 +719,11 @@ BOOL WholeSysDriveEncryption (BOOL bSilent)
 	{
 		BootEncStatus = BootEncObj->GetStatus();
 
-		return (BootEncStatus.ConfiguredEncryptedAreaStart == TC_BOOT_LOADER_AREA_SIZE
+		if (BootEncStatus.BootDriveLength.QuadPart < 1) // paranoid check
-			&& BootEncStatus.ConfiguredEncryptedAreaEnd >= BootEncStatus.BootDriveLength.QuadPart - 1);
+			return FALSE;
+		else
+			return (BootEncStatus.ConfiguredEncryptedAreaStart == TC_BOOT_LOADER_AREA_SIZE
+				&& BootEncStatus.ConfiguredEncryptedAreaEnd >= BootEncStatus.BootDriveLength.QuadPart - 1);
 	}
 	catch (Exception &e)
 	{
@@ -742,9 +745,16 @@ unsigned __int64 GetSysEncDeviceSize (BOOL bSilent)
 	{
 		if (!bSilent)
 			e.Show (MainDlg);
+		return 1;
 	}
 
-	return (BootEncStatus.ConfiguredEncryptedAreaEnd - BootEncStatus.ConfiguredEncryptedAreaStart + 1);
+	if (	BootEncStatus.ConfiguredEncryptedAreaEnd < 0
+		||	BootEncStatus.ConfiguredEncryptedAreaStart < 0
+		||	BootEncStatus.ConfiguredEncryptedAreaEnd < BootEncStatus.ConfiguredEncryptedAreaStart
+		)
+		return 1; // we return 1 to avoid devision by zero
+	else
+		return ((unsigned __int64)(BootEncStatus.ConfiguredEncryptedAreaEnd - BootEncStatus.ConfiguredEncryptedAreaStart)) + 1;
 }
 
 // Returns the current size of the encrypted area of the system drive/partition in bytes
@@ -758,9 +768,16 @@ unsigned __int64 GetSysEncDeviceEncryptedPartSize (BOOL bSilent)
 	{
 		if (!bSilent)
 			e.Show (MainDlg);
+		return 0;
 	}
 
-	return (BootEncStatus.EncryptedAreaEnd - BootEncStatus.EncryptedAreaStart + 1);
+	if (	BootEncStatus.EncryptedAreaEnd < 0 
+		|| BootEncStatus.EncryptedAreaStart < 0
+		|| BootEncStatus.EncryptedAreaEnd < BootEncStatus.EncryptedAreaStart
+		)
+		return 0;
+	else
+		return ((unsigned __int64)(BootEncStatus.EncryptedAreaEnd - BootEncStatus.EncryptedAreaStart)) + 1;
 }
 
 
@@ -2885,14 +2902,19 @@ int GetCipherBlockSizeByDriveNo (int nDosDriveNo)
 
 	if (DeviceIoControl (hDriver, TC_IOCTL_GET_VOLUME_PROPERTIES, &prop, sizeof (prop), &prop, sizeof (prop), &dwResult, NULL))
 	{
-		for (cipherID = EAGetLastCipher (prop.ea);
+		if (	(prop.driveNo == nDosDriveNo)
-			cipherID != 0;
+			&&	(prop.ea >= EAGetFirst() && prop.ea <= EAGetCount())
-			cipherID = EAGetPreviousCipher (prop.ea, cipherID))
+			)
 		{
-			if (blockSize > 0)
+			for (cipherID = EAGetLastCipher (prop.ea);
-				blockSize = min (blockSize, CipherGetBlockSize (cipherID) * 8);
+				cipherID != 0;
-			else
+				cipherID = EAGetPreviousCipher (prop.ea, cipherID))
-				blockSize = CipherGetBlockSize (cipherID) * 8;
+			{
+				if (blockSize > 0)
+					blockSize = min (blockSize, CipherGetBlockSize (cipherID) * 8);
+				else
+					blockSize = CipherGetBlockSize (cipherID) * 8;
+			}
 		}
 	}
 
@@ -2911,7 +2933,13 @@ int GetModeOfOperationByDriveNo (int nDosDriveNo)
 
 	if (DeviceIoControl (hDriver, TC_IOCTL_GET_VOLUME_PROPERTIES, &prop, sizeof (prop), &prop, sizeof (prop), &dwResult, NULL))
 	{
-		return prop.mode;
+		if (	(prop.driveNo == nDosDriveNo)
+			&&	(prop.ea >= EAGetFirst() && prop.ea <= EAGetCount())
+			&&	(prop.mode >= FIRST_MODE_OF_OPERATION_ID && prop.mode < MODE_ENUM_END_ID)
+			)
+		{
+			return prop.mode;
+		}
 	}
 
 	return 0;
@@ -3359,7 +3387,7 @@ BOOL CALLBACK TravelerDlgProc (HWND hwndDlg, UINT msg, WPARAM wParam, LPARAM lPa
 
 			GetDlgItemText (hwndDlg, IDC_DIRECTORY, dstDir, sizeof dstDir);
 			volName[0] = 0;
-			GetDlgItemText (hwndDlg, IDC_VOLUME_NAME, volName + 1, sizeof volName);
+			GetDlgItemText (hwndDlg, IDC_VOLUME_NAME, volName + 1, (sizeof volName) - 1);
 			
 			drive = SendDlgItemMessage (hwndDlg, IDC_DRIVELIST, CB_GETCURSEL, 0, 0);
 			drive = SendDlgItemMessage (hwndDlg, IDC_DRIVELIST, CB_GETITEMDATA, drive, 0);
@@ -3872,9 +3900,9 @@ void __cdecl mountThreadFunction (void *hwndDlgArg)
 static BOOL DismountAll (HWND hwndDlg, BOOL forceUnmount, BOOL interact, int dismountMaxRetries, int dismountAutoRetryDelay)
 {
 	BOOL status = TRUE;
-	MOUNT_LIST_STRUCT mountList;
+	MOUNT_LIST_STRUCT mountList = {0};
 	DWORD dwResult;
-	UNMOUNT_STRUCT unmount;
+	UNMOUNT_STRUCT unmount = {0};
 	BOOL bResult;
 	unsigned __int32 prevMountedDrives = 0;
 	int i;
@@ -3911,6 +3939,17 @@ retry:
 		bResult = DeviceIoControl (hDriver, TC_IOCTL_DISMOUNT_ALL_VOLUMES, &unmount,
 			sizeof (unmount), &unmount, sizeof (unmount), &dwResult, NULL);
 
+		if (	unmount.nDosDriveNo < 0 || unmount.nDosDriveNo > 25 
+				|| (unmount.ignoreOpenFiles != TRUE && unmount.ignoreOpenFiles != FALSE)
+				||	(unmount.HiddenVolumeProtectionTriggered != TRUE && unmount.HiddenVolumeProtectionTriggered != FALSE)
+				||	(unmount.nReturnCode < 0)
+			)
+		{
+			if (bResult)
+				SetLastError (ERROR_INTERNAL_ERROR);
+			bResult = FALSE;			
+		}
+
 		if (bResult == FALSE)
 		{
 			NormalCursor();
@@ -5847,7 +5886,7 @@ BOOL CALLBACK MainDialogProc (HWND hwndDlg, UINT uMsg, WPARAM wParam, LPARAM lPa
 
 					if (IsVolumeDeviceHosted (volp))
 					{
-						OPEN_TEST_STRUCT ots;
+						OPEN_TEST_STRUCT ots = {0};
 
 						if (!OpenDevice (volp, &ots, FALSE))
 						{
@@ -7394,6 +7433,7 @@ int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance, char *lpszComm
 	DialogBoxParamW (hInstance, MAKEINTRESOURCEW (IDD_MOUNT_DLG), NULL, (DLGPROC) MainDialogProc,
 			(LPARAM) lpszCommandLine);
 
+	FinalizeApp ();
 	/* Terminate */
 	return 0;
 }
@@ -7412,7 +7452,7 @@ BOOL TaskBarIconAdd (HWND hwnd)
 	TaskBarIconMutex = CreateMutex (NULL, TRUE, "VeraCryptTaskBarIcon");
 	if (TaskBarIconMutex == NULL || GetLastError () == ERROR_ALREADY_EXISTS)
 	{
-		if (TaskBarIconMutex)
+		if (TaskBarIconMutex != NULL)
 		{
 			CloseHandle(TaskBarIconMutex);
 			TaskBarIconMutex = NULL;
@@ -7516,7 +7556,15 @@ void DismountIdleVolumes ()
 			bResult = DeviceIoControl (hDriver, TC_IOCTL_GET_VOLUME_PROPERTIES, &prop,
 				sizeof (prop), &prop, sizeof (prop), &dwResult, NULL);
 
-			if (bResult)
+			if (	bResult 
+				&&	(	(prop.driveNo == i) && prop.uniqueId >= 0 
+					&& prop.ea >= EAGetFirst() && prop.ea <= EAGetCount()
+					&& prop.mode >= FIRST_MODE_OF_OPERATION_ID && prop.mode <= LAST_MODE_OF_OPERATION 
+					&& prop.pkcs5 >= FIRST_PRF_ID && prop.pkcs5 <= LAST_PRF_ID 
+					&& prop.pkcs5Iterations > 0
+					&& prop.hiddenVolProtection >= 0 && prop.volFormatVersion >= 0
+					)
+				)
 			{
 				if (LastRead[i] == prop.totalBytesRead 
 					&& LastWritten[i] == prop.totalBytesWritten
@@ -9145,9 +9193,9 @@ void AnalyzeKernelMiniDump (HWND hwndDlg)
 			return;
 
 		if (Is64BitOs())
-			sDbgCmd = "msiexec.exe /qb /i http://www.idrix.fr/Root/MSDebug/dbg_amd64_6.11.1.404.msi";
+			sDbgCmd = "msiexec.exe /qb /i https://www.idrix.fr/Root/MSDebug/dbg_amd64_6.11.1.404.msi";
 		else
-			sDbgCmd = "msiexec.exe /qb /i http://www.idrix.fr/Root/MSDebug/dbg_x86_6.11.1.404.msi";
+			sDbgCmd = "msiexec.exe /qb /i https://www.idrix.fr/Root/MSDebug/dbg_x86_6.11.1.404.msi";
 		
 		if (!CreateProcess (NULL, (LPSTR) sDbgCmd.c_str(),
 			NULL, NULL, FALSE, 0, NULL, NULL, &startupInfo, &procInfo))
@@ -9253,14 +9301,31 @@ void AnalyzeKernelMiniDump (HWND hwndDlg)
 	CloseHandle (hChildStdoutWrite);
 
 	string output;
+	BOOL bIsValidResponse = TRUE;
 
 	while (TRUE) 
 	{ 
-		DWORD bytesReceived;
+		DWORD bytesReceived = 0, i;
-		char pipeBuffer [4096];
+		char pipeBuffer [4096] = {0};
+		
+		unsigned char uc;
 
 		if (!ReadFile (hChildStdoutRead, pipeBuffer, sizeof (pipeBuffer), &bytesReceived, NULL)) 
-			break; 
+			break;
+
+		/* check if the buffer contains printable characters only*/
+		for (i = 0; i < bytesReceived; i++)
+		{
+			uc =  (unsigned char) pipeBuffer [i];
+			if ( uc >= 0x7f || uc < 0x20) // A non-ASCII or non-printable character?
+			{
+				bIsValidResponse = FALSE;
+				break;
+			}
+		}
+
+		if (!bIsValidResponse)
+			break;
 
 		output.insert (output.size(), pipeBuffer, bytesReceived);
 	}
@@ -9269,6 +9334,12 @@ void AnalyzeKernelMiniDump (HWND hwndDlg)
 
 	NormalCursor();
 
+	if (!bIsValidResponse)
+	{
+		Error ("ERR_PARAMETER_INCORRECT", hwndDlg);
+		return;
+	}
+
 	bool otherDriver = (StringToUpperCase (output).find (StringToUpperCase (TC_APP_NAME)) == string::npos);
 
 	size_t p, p2;

src/Setup/Dir.c

@@ -17,6 +17,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <errno.h>
+#include <Strsafe.h>
 
 #include "Dir.h"
 
@@ -28,7 +29,7 @@ mkfulldir (char *oriPath, BOOL bCheckonly)
 	char *uniq_file;
 	char path [TC_MAX_PATH];
 
-	strcpy (path, oriPath);
+	StringCbCopyA (path, TC_MAX_PATH, oriPath);
 
 	if (strlen (path) == 3 && path[1] == ':')
 		goto is_root;	/* keep final slash in root if present */
@@ -63,7 +64,7 @@ mkfulldir_internal (char *path)
 	static char tokpath[_MAX_PATH];
 	static char trail[_MAX_PATH];
 
-	strcpy (tokpath, path);
+	StringCbCopyA (tokpath, _MAX_PATH, path);
 	trail[0] = '\0';
 
 	token = strtok (tokpath, "\\/");
@@ -75,13 +76,13 @@ mkfulldir_internal (char *path)
 		trail[2] = '\0';
 		if (token)
 		{
-			strcat (trail, token);
+			StringCbCatA (trail, _MAX_PATH, token);
-			strcat (trail, "\\");
+			StringCbCatA (trail, _MAX_PATH, "\\");
 			token = strtok (NULL, "\\/");
 			if (token)
 			{		/* get share name */
-				strcat (trail, token);
+				StringCbCatA (trail, _MAX_PATH, token);
-				strcat (trail, "\\");
+				StringCbCatA (trail, _MAX_PATH, "\\");
 			}
 			token = strtok (NULL, "\\/");
 		}
@@ -89,17 +90,17 @@ mkfulldir_internal (char *path)
 
 	if (tokpath[1] == ':')
 	{			/* drive letter */
-		strcat (trail, tokpath);
+		StringCbCatA (trail, _MAX_PATH, tokpath);
-		strcat (trail, "\\");
+		StringCbCatA (trail, _MAX_PATH, "\\");
 		token = strtok (NULL, "\\/");
 	}
 
 	while (token != NULL)
 	{
 		int x;
-		strcat (trail, token);
+		StringCbCatA (trail, _MAX_PATH, token);
 		x = _mkdir (trail);
-		strcat (trail, "\\");
+		StringCbCatA (trail, _MAX_PATH, "\\");
 		token = strtok (NULL, "\\/");
 	}
 

src/Setup/Setup.c

@@ -251,11 +251,12 @@ void IconMessage (HWND hwndDlg, char *txt)
 void DetermineUpgradeDowngradeStatus (BOOL bCloseDriverHandle, LONG *driverVersionPtr)
 {
 	LONG driverVersion = VERSION_NUM;
+	int status = 0;
 
 	if (hDriver == INVALID_HANDLE_VALUE)
-		DriverAttach();
+		status = DriverAttach();
 
-	if (hDriver != INVALID_HANDLE_VALUE)
+	if ((status == 0) && (hDriver != INVALID_HANDLE_VALUE))
 	{
 		DWORD dwResult;
 		BOOL bResult = DeviceIoControl (hDriver, TC_IOCTL_GET_DRIVER_VERSION, NULL, 0, &driverVersion, sizeof (driverVersion), &dwResult, NULL);
@@ -745,7 +746,6 @@ BOOL DoApplicationDataUninstall (HWND hwndDlg)
 
 BOOL DoRegUninstall (HWND hwndDlg, BOOL bRemoveDeprecated)
 {
-	BOOL bOK = FALSE;
 	char regk [64];
 
 	// Unregister COM servers
@@ -775,17 +775,7 @@ BOOL DoRegUninstall (HWND hwndDlg, BOOL bRemoveDeprecated)
 		SHChangeNotify (SHCNE_ASSOCCHANGED, SHCNF_IDLIST, NULL, NULL);
 	}
 
-	bOK = TRUE;
+	return TRUE;
-
-	if (bOK == FALSE && GetLastError ()!= ERROR_NO_TOKEN && GetLastError ()!= ERROR_FILE_NOT_FOUND
-	    && GetLastError ()!= ERROR_PATH_NOT_FOUND)
-	{
-		handleWin32Error (hwndDlg);
-	}
-	else
-		bOK = TRUE;
-
-	return bOK;
 }
 
 
@@ -876,10 +866,16 @@ try_delete:
 		StatusMessageParam (hwndDlg, "REMOVING", lpszService);
 
 	if (hService != NULL)
+	{
 		CloseServiceHandle (hService);
+		hService = NULL;
+	}
 
 	if (hManager != NULL)
+	{
 		CloseServiceHandle (hManager);
+		hManager = NULL;
+	}
 
 	hManager = OpenSCManager (NULL, NULL, SC_MANAGER_ALL_ACCESS);
 	if (hManager == NULL)
@@ -897,6 +893,8 @@ try_delete:
 			// Second try for an eventual no-install driver instance
 			CloseServiceHandle (hService);
 			CloseServiceHandle (hManager);
+			hService = NULL;
+			hManager = NULL;
 
 			Sleep(1000);
 			firstTry = FALSE;
@@ -1944,6 +1942,7 @@ int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance, char *lpszComm
 	if (IsAdmin () != TRUE)
 		if (MessageBoxW (NULL, GetString ("SETUP_ADMIN"), lpszTitle, MB_YESNO | MB_ICONQUESTION) != IDYES)
 		{
+			FinalizeApp ();
 			exit (1);
 		}
 
@@ -2009,6 +2008,7 @@ int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance, char *lpszComm
 			else if (!bDevm)
 			{
 				MessageBox (NULL, "Error: This installer file does not contain any compressed files.\n\nTo create a self-extracting installation package (with embedded compressed files), run:\n\"VeraCrypt Setup.exe\" /p", "VeraCrypt", MB_ICONERROR | MB_SETFOREGROUND | MB_TOPMOST);
+				FinalizeApp ();
 				exit (1);
 			}
 
@@ -2028,6 +2028,7 @@ int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance, char *lpszComm
 					bUninstall = TRUE;
 					break;
 				default:
+					FinalizeApp ();
 					exit (1);
 				}
 			}
@@ -2076,6 +2077,6 @@ int WINAPI WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance, char *lpszComm
 			}
 		}
 	}
-
+	FinalizeApp ();
 	return 0;
 }