diff options
| author | Mounir IDRASSI <mounir.idrassi@idrix.fr> | 2015-02-08 23:46:04 +0100 |
|---|---|---|
| committer | Mounir IDRASSI <mounir.idrassi@idrix.fr> | 2015-02-09 11:01:21 +0100 |
| commit | d5f34ad49d345803767d4a1166d764f9f8485541 (patch) | |
| tree | a101ab51e55dffdbcdef4c15596fd9418df421cd /src | |
| parent | 608e86c7bc962f369003d9d05d4402f9da273f0c (diff) | |
| download | VeraCrypt-d5f34ad49d345803767d4a1166d764f9f8485541.tar.gz VeraCrypt-d5f34ad49d345803767d4a1166d764f9f8485541.zip | |
Static Code Analysis: Avoid over-flaw in arithmetic operations by adding more checks. Add extra checks. Solve various issues.
Diffstat (limited to 'src')
| -rw-r--r-- | src/Format/InPlace.c | 12 | ||||
| -rw-r--r-- | src/Format/Tcformat.c | 14 | ||||
| -rw-r--r-- | src/Mount/MainCom.cpp | 35 | ||||
| -rw-r--r-- | src/Mount/Mount.c | 117 | ||||
| -rw-r--r-- | src/Setup/Dir.c | 21 | ||||
| -rw-r--r-- | src/Setup/Setup.c | 31 |
6 files changed, 172 insertions, 58 deletions
diff --git a/src/Format/InPlace.c b/src/Format/InPlace.c index 4c5491e3..3998c2a5 100644 --- a/src/Format/InPlace.c +++ b/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;
diff --git a/src/Format/Tcformat.c b/src/Format/Tcformat.c index 4984e6cc..995222de 100644 --- a/src/Format/Tcformat.c +++ b/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)
- {
- burn ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
- VirtualUnlock ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
- free ((LPVOID) volParams);
- volParams = NULL;
- }
+
+ // volParams is ensured to be non NULL at this stage
+ burn ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
+ VirtualUnlock ((LPVOID) volParams, sizeof(FORMAT_VOL_PARAMETERS));
+ free ((LPVOID) volParams);
+ 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;
}
diff --git a/src/Mount/MainCom.cpp b/src/Mount/MainCom.cpp index b2dfe89c..5a43d36f 100644 --- a/src/Mount/MainCom.cpp +++ b/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
diff --git a/src/Mount/Mount.c b/src/Mount/Mount.c index c6feb3b5..eb14d76e 100644 --- a/src/Mount/Mount.c +++ b/src/Mount/Mount.c @@ -719,8 +719,11 @@ BOOL WholeSysDriveEncryption (BOOL bSilent) {
BootEncStatus = BootEncObj->GetStatus();
- return (BootEncStatus.ConfiguredEncryptedAreaStart == TC_BOOT_LOADER_AREA_SIZE
- && BootEncStatus.ConfiguredEncryptedAreaEnd >= BootEncStatus.BootDriveLength.QuadPart - 1);
+ if (BootEncStatus.BootDriveLength.QuadPart < 1) // paranoid check
+ 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);
- cipherID != 0;
- cipherID = EAGetPreviousCipher (prop.ea, cipherID))
+ if ( (prop.driveNo == nDosDriveNo)
+ && (prop.ea >= EAGetFirst() && prop.ea <= EAGetCount())
+ )
{
- if (blockSize > 0)
- blockSize = min (blockSize, CipherGetBlockSize (cipherID) * 8);
- else
- blockSize = CipherGetBlockSize (cipherID) * 8;
+ for (cipherID = EAGetLastCipher (prop.ea);
+ cipherID != 0;
+ cipherID = EAGetPreviousCipher (prop.ea, cipherID))
+ {
+ 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;
- char pipeBuffer [4096];
+ DWORD bytesReceived = 0, i;
+ 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;
diff --git a/src/Setup/Dir.c b/src/Setup/Dir.c index a0380ef2..ec530df4 100644 --- a/src/Setup/Dir.c +++ b/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);
- strcat (trail, "\\");
+ StringCbCatA (trail, _MAX_PATH, token);
+ StringCbCatA (trail, _MAX_PATH, "\\");
token = strtok (NULL, "\\/");
if (token)
{ /* get share name */
- strcat (trail, token);
- strcat (trail, "\\");
+ StringCbCatA (trail, _MAX_PATH, token);
+ StringCbCatA (trail, _MAX_PATH, "\\");
}
token = strtok (NULL, "\\/");
}
@@ -89,17 +90,17 @@ mkfulldir_internal (char *path) if (tokpath[1] == ':')
{ /* drive letter */
- strcat (trail, tokpath);
- strcat (trail, "\\");
+ StringCbCatA (trail, _MAX_PATH, tokpath);
+ 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, "\\/");
}
diff --git a/src/Setup/Setup.c b/src/Setup/Setup.c index 9f5fbfdd..dc8123c6 100644 --- a/src/Setup/Setup.c +++ b/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;
-
- if (bOK == FALSE && GetLastError ()!= ERROR_NO_TOKEN && GetLastError ()!= ERROR_FILE_NOT_FOUND
- && GetLastError ()!= ERROR_PATH_NOT_FOUND)
- {
- handleWin32Error (hwndDlg);
- }
- else
- bOK = TRUE;
-
- return bOK;
+ return TRUE;
}
@@ -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;
}
|