Windows: Add various checks to address Coverity reported issues.

This commit is contained in:
Mounir IDRASSI 2022-03-26 20:03:19 +01:00
parent a0809fe85c
commit 762065917f
No known key found for this signature in database
GPG Key ID: 02C30AE90FAE4A6F
11 changed files with 105 additions and 28 deletions

View File

@ -611,6 +611,7 @@ char *LoadFile (const wchar_t *fileName, DWORD *size)
char *buf; char *buf;
DWORD fileSize = INVALID_FILE_SIZE; DWORD fileSize = INVALID_FILE_SIZE;
HANDLE h = CreateFile (fileName, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL); HANDLE h = CreateFile (fileName, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, 0, NULL);
*size = 0;
if (h == INVALID_HANDLE_VALUE) if (h == INVALID_HANDLE_VALUE)
return NULL; return NULL;
@ -620,8 +621,7 @@ char *LoadFile (const wchar_t *fileName, DWORD *size)
return NULL; return NULL;
} }
*size = fileSize; buf = (char *) calloc (fileSize + 1, 1);
buf = (char *) calloc (*size + 1, 1);
if (buf == NULL) if (buf == NULL)
{ {
@ -629,11 +629,15 @@ char *LoadFile (const wchar_t *fileName, DWORD *size)
return NULL; return NULL;
} }
if (!ReadFile (h, buf, *size, size, NULL)) if (!ReadFile (h, buf, fileSize, size, NULL))
{ {
free (buf); free (buf);
buf = NULL; 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); CloseHandle (h);
return buf; return buf;

View File

@ -611,7 +611,8 @@ char *GetPreferredLangId ()
void SetPreferredLangId (char *langId) void SetPreferredLangId (char *langId)
{ {
StringCbCopyA (PreferredLangId, sizeof(PreferredLangId), langId); if (strlen(langId) < sizeof(PreferredLangId))
StringCbCopyA (PreferredLangId, sizeof(PreferredLangId), langId);
} }

View File

@ -1519,12 +1519,20 @@ BOOL test_hmac_sha256 ()
for (i = 0; i < sizeof (hmac_sha256_test_data) / sizeof(char *); i++) for (i = 0; i < sizeof (hmac_sha256_test_data) / sizeof(char *); i++)
{ {
char digest[1024]; /* large enough to hold digets and test vector inputs */ 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])); size_t dataLen = 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 (dataLen <= sizeof(digest))
if (memcmp (digest, hmac_sha256_test_vectors[i], SHA256_DIGESTSIZE) != 0) {
return FALSE; 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 else
nTestsPerformed++; {
return FALSE;
}
} }
return (nTestsPerformed == 6); return (nTestsPerformed == 6);
@ -1538,12 +1546,20 @@ BOOL test_hmac_sha512 ()
for (i = 0; i < sizeof (hmac_sha512_test_data) / sizeof(char *); i++) for (i = 0; i < sizeof (hmac_sha512_test_data) / sizeof(char *); i++)
{ {
char digest[1024]; /* large enough to hold digets and test vector inputs */ 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])); size_t dataLen = 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 (dataLen <= sizeof(digest))
if (memcmp (digest, hmac_sha512_test_vectors[i], SHA512_DIGESTSIZE) != 0) {
return FALSE; 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 else
nTestsPerformed++; {
return FALSE;
}
} }
return (nTestsPerformed == 6); return (nTestsPerformed == 6);
@ -1557,12 +1573,20 @@ BOOL test_hmac_blake2s ()
for (i = 0; i < sizeof (hmac_blake2s_test_data) / sizeof(char *); i++) for (i = 0; i < sizeof (hmac_blake2s_test_data) / sizeof(char *); i++)
{ {
char digest[1024]; /* large enough to hold digets and test vector inputs */ 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])); size_t dataLen = 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 (dataLen <= sizeof(digest))
if (memcmp (digest, hmac_blake2s_test_vectors[i], BLAKE2S_DIGESTSIZE) != 0) {
return FALSE; 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 else
nTestsPerformed++; {
return FALSE;
}
} }
return (nTestsPerformed == 6); return (nTestsPerformed == 6);

View File

@ -280,7 +280,7 @@ static int Detect_MS_HyperV_AES ()
// when Hyper-V is enabled on older versions of Windows Server (i.e. 2008 R2), the AES-NI capability // 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. // 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 // 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]; char HvProductName[13];
CpuId(0x40000000, cpuid); CpuId(0x40000000, cpuid);
@ -348,7 +348,7 @@ void DetectX86Features()
g_hasISSE = 1; g_hasISSE = 1;
else else
{ {
uint32 cpuid2[4]; uint32 cpuid2[4] = {0};
CpuId(0x080000000, cpuid2); CpuId(0x080000000, cpuid2);
if (cpuid2[0] >= 0x080000001) if (cpuid2[0] >= 0x080000001)
{ {

View File

@ -442,6 +442,12 @@ int ExtendFileSystem (HWND hwndDlg , wchar_t *lpszVolume, Password *pVolumePassw
goto 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"); DebugAddProgressDlgStatus (hwndDlg, L"Extending file system ...\r\n");
// extend volume // extend volume

View File

@ -89,6 +89,8 @@ static __int64 NewFileSysSizeAfterShrink (HANDLE dev, const wchar_t *devicePath,
} }
if ( (ntfsVolData.NumberSectors.QuadPart <= 0) if ( (ntfsVolData.NumberSectors.QuadPart <= 0)
|| (ntfsVolData.BytesPerSector == 0)
|| (ntfsVolData.BytesPerSector >= (DWORD) UINT_MAX)
|| (ntfsVolData.NumberSectors.QuadPart > (INT64_MAX / (__int64) ntfsVolData.BytesPerSector)) // overflow test || (ntfsVolData.NumberSectors.QuadPart > (INT64_MAX / (__int64) ntfsVolData.BytesPerSector)) // overflow test
) )
{ {

View File

@ -9756,11 +9756,18 @@ int AnalyzeHiddenVolumeHost (HWND hwndDlg, int *driveNo, __int64 hiddenVolHostSi
// The map will be scanned to determine the size of the uninterrupted block of free // 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. // 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. // The value will then be used to determine the maximum possible size of the hidden volume.
if (*realClusterSize > 0)
return ScanVolClusterBitmap (hwndDlg, {
driveNo, return ScanVolClusterBitmap (hwndDlg,
hiddenVolHostSize / *realClusterSize, driveNo,
pnbrFreeClusters); hiddenVolHostSize / *realClusterSize,
pnbrFreeClusters);
}
else
{
// should never happen
return -1;
}
} }
else if (!wcsncmp (szFileSystemNameBuffer, L"NTFS", 4) || !_wcsnicmp (szFileSystemNameBuffer, L"exFAT", 5)) else if (!wcsncmp (szFileSystemNameBuffer, L"NTFS", 4) || !_wcsnicmp (szFileSystemNameBuffer, L"exFAT", 5))
{ {

View File

@ -6583,6 +6583,14 @@ static void ShowSystemEncryptionStatus (HWND hwndDlg)
if (GetAsyncKeyState (VK_SHIFT) < 0 && GetAsyncKeyState (VK_CONTROL) < 0) if (GetAsyncKeyState (VK_SHIFT) < 0 && GetAsyncKeyState (VK_CONTROL) < 0)
{ {
// Ctrl+Shift held (for debugging purposes) // 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" 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" "HiddenSystem: %d\nHiddenSystemPartitionStart: %I64d\n"
@ -6600,7 +6608,7 @@ static void ShowSystemEncryptionStatus (HWND hwndDlg)
BootEncStatus.ConfiguredEncryptedAreaEnd, BootEncStatus.ConfiguredEncryptedAreaEnd,
BootEncStatus.EncryptedAreaStart, BootEncStatus.EncryptedAreaStart,
BootEncStatus.EncryptedAreaEnd, BootEncStatus.EncryptedAreaEnd,
!BootEncStatus.DriveEncrypted ? 0 : (BootEncStatus.EncryptedAreaEnd + 1 - BootEncStatus.EncryptedAreaStart) * 100I64 / (BootEncStatus.ConfiguredEncryptedAreaEnd + 1 - BootEncStatus.ConfiguredEncryptedAreaStart)); encryptedRatio);
} }
if (!BootEncStatus.DriveEncrypted && !BootEncStatus.DriveMounted) if (!BootEncStatus.DriveEncrypted && !BootEncStatus.DriveMounted)

View File

@ -31,6 +31,12 @@ mkfulldir (wchar_t *oriPath, BOOL bCheckonly)
wchar_t *uniq_file; wchar_t *uniq_file;
wchar_t path [TC_MAX_PATH]; 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); StringCbCopyW (path, TC_MAX_PATH, oriPath);
if (wcslen (path) == 3 && path[1] == L':') if (wcslen (path) == 3 && path[1] == L':')
@ -66,6 +72,12 @@ mkfulldir_internal (wchar_t *path)
static wchar_t tokpath[_MAX_PATH]; static wchar_t tokpath[_MAX_PATH];
static wchar_t trail[_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); StringCbCopyW (tokpath, _MAX_PATH, path);
trail[0] = L'\0'; trail[0] = L'\0';

View File

@ -819,7 +819,8 @@ BOOL DoFilesInstall (HWND hwndDlg, wchar_t *szDestDir)
if (Is64BitOs ()) if (Is64BitOs ())
driver64 = TRUE; driver64 = TRUE;
GetSystemDirectory (szDir, ARRAYSIZE (szDir)); if (!GetSystemDirectory (szDir, ARRAYSIZE (szDir)))
StringCbCopyW(szDir, sizeof(szDir), L"C:\\Windows\\System32");
x = wcslen (szDir); x = wcslen (szDir);
if (szDir[x - 1] != L'\\') if (szDir[x - 1] != L'\\')

View File

@ -31,6 +31,12 @@ mkfulldir (wchar_t *oriPath, BOOL bCheckonly)
wchar_t *uniq_file; wchar_t *uniq_file;
wchar_t path [TC_MAX_PATH]; 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); StringCbCopyW (path, TC_MAX_PATH, oriPath);
if (wcslen (path) == 3 && path[1] == L':') if (wcslen (path) == 3 && path[1] == L':')
@ -66,6 +72,12 @@ mkfulldir_internal (wchar_t *path)
static wchar_t tokpath[_MAX_PATH]; static wchar_t tokpath[_MAX_PATH];
static wchar_t trail[_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); StringCbCopyW (tokpath, _MAX_PATH, path);
trail[0] = L'\0'; trail[0] = L'\0';