[misc] fix various Coverity issues

* update DLL load/unload
* fix resources not being freed and potentially unsafe calls
* add extra checks
This commit is contained in:
Pete Batard 2014-05-09 22:05:25 +01:00
parent 9aa308213d
commit 266599e6fd
12 changed files with 68 additions and 36 deletions

View File

@ -988,7 +988,7 @@ BOOL SetDOSLocale(const char* path, BOOL bFreeDOS)
if ((cp == 437) && (strcmp(kb, "us") == 0)) {
// Nothing much to do if US/US - just notify in autoexec.bat
strcpy(filename, path);
safe_strcpy(filename, sizeof(filename), path);
safe_strcat(filename, sizeof(filename), "\\AUTOEXEC.BAT");
fd = fopen(filename, "w+");
if (fd == NULL) {
@ -1004,7 +1004,7 @@ BOOL SetDOSLocale(const char* path, BOOL bFreeDOS)
}
// CONFIG.SYS
strcpy(filename, path);
safe_strcpy(filename, sizeof(filename), path);
safe_strcat(filename, sizeof(filename), "\\CONFIG.SYS");
fd = fopen(filename, "w+");
if (fd == NULL) {
@ -1029,7 +1029,7 @@ BOOL SetDOSLocale(const char* path, BOOL bFreeDOS)
uprintf("Successfully wrote 'CONFIG.SYS'\n");
// AUTOEXEC.BAT
strcpy(filename, path);
safe_strcpy(filename, sizeof(filename), path);
safe_strcat(filename, sizeof(filename), "\\AUTOEXEC.BAT");
fd = fopen(filename, "w+");
if (fd == NULL) {

View File

@ -188,7 +188,7 @@ char* GetLogicalName(DWORD DriveIndex, BOOL bKeepTrailingBackslash, BOOL bSilent
}
for (j=0; (j<ARRAYSIZE(ignore_device)) &&
(safe_strnicmp(path, ignore_device[j], safe_strlen(ignore_device[j])) != 0); j++);
(_strnicmp(path, ignore_device[j], safe_strlen(ignore_device[j])) != 0); j++);
if (j < ARRAYSIZE(ignore_device)) {
suprintf("Skipping GUID volume for '%s'\n", path);
continue;
@ -258,7 +258,7 @@ HANDLE GetLogicalHandle(DWORD DriveIndex, BOOL bWriteAccess, BOOL bLockDrive)
}
hLogical = GetHandle(LogicalPath, bWriteAccess, bLockDrive);
safe_free(LogicalPath);
free(LogicalPath);
return hLogical;
}
@ -453,7 +453,7 @@ BOOL GetDriveLabel(DWORD DriveIndex, char* letters, char** label)
safe_closehandle(hPhysical);
if (AutorunLabel != NULL) {
uprintf("Using autorun.inf label for drive %c: '%s'\n", letters[0], AutorunLabel);
strncpy(VolumeLabel, AutorunLabel, sizeof(VolumeLabel));
safe_strcpy(VolumeLabel, sizeof(VolumeLabel), AutorunLabel);
safe_free(AutorunLabel);
*label = VolumeLabel;
} else if (GetVolumeInformationW(wDrivePath, wVolumeLabel, ARRAYSIZE(wVolumeLabel),
@ -626,6 +626,7 @@ int GetDrivePartitionData(DWORD DriveIndex, char* FileSystemName, DWORD FileSyst
NULL, 0, layout, sizeof(layout), &size, NULL );
if (!r || size <= 0) {
uprintf("Could not get layout for drive 0x%02x: %s\n", DriveIndex, WindowsErrorString());
safe_closehandle(hPhysical);
return 0;
}

View File

@ -461,13 +461,14 @@ static BOOL FormatFAT32(DWORD DriveIndex)
if (qTotalSectors >= 0xffffffff) {
// This is a more fundamental limitation on FAT32 - the total sector count in the root dir
// ís 32bit. With a bit of creativity, FAT32 could be extended to handle at least 2^28 clusters
// is 32bit. With a bit of creativity, FAT32 could be extended to handle at least 2^28 clusters
// There would need to be an extra field in the FSInfo sector, and the old sector count could
// be set to 0xffffffff. This is non standard though, the Windows FAT driver FASTFAT.SYS won't
// understand this. Perhaps a future version of FAT32 and FASTFAT will handle this.
die("This drive is too big for FAT32 - max 2TB supported\n", APPERR(ERROR_INVALID_VOLUME_SIZE));
}
// coverity[tainted_data]
pFAT32BootSect = (FAT_BOOTSECTOR32*) calloc(BytesPerSect, 1);
pFAT32FsInfo = (FAT_FSINFO*) calloc(BytesPerSect, 1);
pFirstSectOfFat = (DWORD*) calloc(BytesPerSect, 1);
@ -923,7 +924,8 @@ static BOOL WriteMBR(HANDLE hPhysicalDrive)
}
// Tell the system we've updated the disk properties
DeviceIoControl(hPhysicalDrive, IOCTL_DISK_UPDATE_PROPERTIES, NULL, 0, NULL, 0, &size, NULL );
if (!DeviceIoControl(hPhysicalDrive, IOCTL_DISK_UPDATE_PROPERTIES, NULL, 0, NULL, 0, &size, NULL))
uprintf("Failed to notify system about disk properties update: %s\n", WindowsErrorString());
out:
safe_free(buf);

View File

@ -225,7 +225,7 @@ static int udf_extract_files(udf_t *p_udf, udf_dirent_t *p_udf_dirent, const cha
goto out;
}
if (udf_is_dir(p_udf_dirent)) {
if (!scan_only) _mkdirU(psz_fullpath);
if (!scan_only) IGNORE_RETVAL(_mkdirU(psz_fullpath));
p_udf_dirent2 = udf_opendir(p_udf_dirent);
if (p_udf_dirent2 != NULL) {
if (udf_extract_files(p_udf, p_udf_dirent2, &psz_fullpath[strlen(psz_extract_dir)]))
@ -376,7 +376,7 @@ static int iso_extract_files(iso9660_t* p_iso, const char *psz_path)
iso9660_name_translate_ext(p_statbuf->filename, psz_basename, i_joliet_level);
}
if (p_statbuf->type == _STAT_DIR) {
if (!scan_only) _mkdirU(psz_fullpath);
if (!scan_only) IGNORE_RETVAL(_mkdirU(psz_fullpath));
if (iso_extract_files(p_iso, psz_iso_name))
goto out;
} else {

View File

@ -276,7 +276,8 @@ DWORD DownloadFile(const char* url, const char* file, HWND hProgressDialog)
PrintStatus(0, FALSE, MSG_240, &file[last_slash]);
uprintf("Downloading '%s' from %s\n", &file[last_slash], url);
if (!InternetCrackUrlA(url, (DWORD)safe_strlen(url), 0, &UrlParts)) {
if ( (!InternetCrackUrlA(url, (DWORD)safe_strlen(url), 0, &UrlParts))
|| (UrlParts.lpszHostName == NULL) || (UrlParts.lpszUrlPath == NULL)) {
uprintf("Unable to decode URL: %s\n", WinInetErrorString());
goto out;
}

View File

@ -306,8 +306,7 @@ BOOL get_supported_locales(const char* filename)
case LC_ATTRIBUTES:
if (last_lcmd == NULL) {
luprint("[a]ttributes cannot precede [l]ocale");
}
for(j=0; lcmd->txt[0][j] != 0; j++) {
} else for(j=0; lcmd->txt[0][j] != 0; j++) {
for (k=0; k<ARRAYSIZE(attr_parse); k++) {
if (attr_parse[k].c == lcmd->txt[0][j]) {
// Repurpose ctrl_id as an attributes mask
@ -438,7 +437,10 @@ BOOL get_loc_data_file(const char* filename, loc_cmd* lcmd)
goto out;
}
fseek(fd, offset, SEEK_SET);
if (fseek(fd, offset, SEEK_SET) != 0) {
uprintf("localization: could not rewind\n");
goto out;
}
do { // custom readline handling for string collation, realloc, line numbers, etc.
c = getc(fd);
@ -547,7 +549,10 @@ BOOL get_loc_data_file(const char* filename, loc_cmd* lcmd)
out:
// Don't close on a reentrant call
if (reentrant) {
fseek(fd, cur_offset, SEEK_SET);
if ((cur_offset < 0) || (fseek(fd, cur_offset, SEEK_SET) != 0)) {
uprintf("localization: unable to reset reentrant position\n");
ret = FALSE;
}
loc_line_nr = old_loc_line_nr;
} else if (fd != NULL) {
fclose(fd);
@ -718,7 +723,7 @@ static __inline char* get_sanitized_token_data_buffer(const char* token, unsigne
size_t i;
char* data = get_token_data_buffer(token, n, buffer, buffer_size);
if (data != NULL) {
for (i=0; i<safe_strlen(data); i++) {
for (i=0; i<strlen(data); i++) {
if ((data[i] == '\\') && (data[i+1] == 'n')) {
data[i] = '\r';
data[i+1] = '\n';
@ -821,7 +826,10 @@ char* insert_section_data(const char* filename, const char* section, const char*
goto out;
}
// Check the input file's BOM and create an output file with the same
fread(&bom, sizeof(bom), 1, fd_in);
if (fread(&bom, sizeof(bom), 1, fd_in) != 1) {
uprintf("Could not read file '%s'\n", filename);
goto out;
}
switch(bom) {
case 0xFEFF:
mode = 2; // UTF-16 (LE)
@ -897,8 +905,9 @@ out:
if (fd_in != NULL) fclose(fd_in);
if (fd_out != NULL) fclose(fd_out);
}
}
_wunlink(wtmpname);
}
if (wtmpname != NULL)
_wunlink(wtmpname);
safe_free(wfilename);
safe_free(wtmpname);
safe_free(wsection);
@ -957,7 +966,10 @@ char* replace_in_token_data(const char* filename, const char* token, const char*
goto out;
}
// Check the input file's BOM and create an output file with the same
fread(&bom, sizeof(bom), 1, fd_in);
if (fread(&bom, sizeof(bom), 1, fd_in) != 1) {
uprintf("Could not read file '%s'\n", filename);
goto out;
}
switch(bom) {
case 0xFEFF:
mode = 2; // UTF-16 (LE)
@ -1045,7 +1057,8 @@ out:
if (fd_out != NULL) fclose(fd_out);
}
}
_wunlink(wtmpname);
if (wtmpname != NULL)
_wunlink(wtmpname);
safe_free(wfilename);
safe_free(wtmpname);
safe_free(wtoken);

View File

@ -130,6 +130,7 @@ char msgbox[1024], msgbox_title[32];
/*
* Globals
*/
OPEN_LIBRARIES_TRACKING_VARS;
HINSTANCE hMainInstance;
HWND hMainDialog;
char szFolderPath[MAX_PATH], app_dir[MAX_PATH];
@ -779,7 +780,7 @@ static BOOL GetUSBDevices(DWORD devnum)
devint_detail_data = (PSP_DEVICE_INTERFACE_DETAIL_DATA_A)calloc(1, size);
if (devint_detail_data == NULL) {
uprintf("Unable to allocate data for SP_DEVICE_INTERFACE_DETAIL_DATA\n");
return FALSE;
continue;
}
devint_detail_data->cbSize = sizeof(SP_DEVICE_INTERFACE_DETAIL_DATA_A);
} else {
@ -979,7 +980,7 @@ void UpdateProgress(int op, float percent)
{
int pos;
if ((op < 0) || (op > OP_MAX)) {
if ((op < 0) || (op >= OP_MAX)) {
duprintf("UpdateProgress: invalid op %d\n", op);
return;
}
@ -1649,7 +1650,7 @@ void InitDialog(HWND hDlg)
CheckDlgButton(hDlg, IDC_SET_ICON, BST_CHECKED);
// Load system icons (NB: Use the excellent http://www.nirsoft.net/utils/iconsext.html to find icon IDs)
hDllInst = LoadLibraryA("shell32.dll");
hDllInst = GetDLLHandle("shell32.dll");
hIconDisc = (HICON)LoadImage(hDllInst, MAKEINTRESOURCE(12), IMAGE_ICON, s16, s16, LR_DEFAULTCOLOR|LR_SHARED);
hIconLang = (HICON)LoadImage(hDllInst, MAKEINTRESOURCE(244), IMAGE_ICON, s16, s16, LR_DEFAULTCOLOR|LR_SHARED);
if (nWindowsVersion >= WINDOWS_VISTA) {
@ -2461,7 +2462,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
IGNORE_RETVAL(CoInitializeEx(NULL, COINIT_APARTMENTTHREADED));
// Some dialogs have Rich Edit controls and won't display without this
if (LoadLibraryA("Riched20.dll") == NULL) {
if (GetDLLHandle("Riched20.dll") == NULL) {
uprintf("Could not load RichEdit library - some dialogs may not display: %s\n", WindowsErrorString());
}
@ -2643,6 +2644,7 @@ out:
if (attached_console)
DetachConsole();
CloseHandle(mutex);
OPEN_LIBRARIES_CLOSE_ALL;
uprintf("*** " APPLICATION_NAME " exit ***\n");
#ifdef _CRTDBG_MAP_ALLOC
_CrtDumpMemoryLeaks();

View File

@ -400,11 +400,23 @@ extern void StrArrayDestroy(StrArray* arr);
* pfFormatEx = (FormatEx_t) GetProcAddress(GetDLLHandle("fmifs"), "FormatEx");
* to make it accessible.
*/
#define MAX_LIBRARY_HANDLES 32
extern HMODULE OpenLibraryHandle[MAX_LIBRARY_HANDLES];
extern uint16_t OpenLibraryHandleSize;
#define OPEN_LIBRARIES_TRACKING_VARS HMODULE OpenLibraryHandle[MAX_LIBRARY_HANDLES]; uint16_t OpenLibraryHandleSize = 0
#define OPEN_LIBRARIES_CLOSE_ALL while(OpenLibraryHandleSize > 0) FreeLibrary(OpenLibraryHandle[--OpenLibraryHandleSize])
static __inline HMODULE GetDLLHandle(char* szDLLName)
{
HMODULE h = NULL;
if ((h = GetModuleHandleA(szDLLName)) == NULL)
h = LoadLibraryA(szDLLName);
if ((h = GetModuleHandleA(szDLLName)) == NULL) {
if (OpenLibraryHandleSize >= MAX_LIBRARY_HANDLES) {
uprintf("Error: MAX_LIBRARY_HANDLES is too small\n");
} else {
h = LoadLibraryA(szDLLName);
if (h != NULL)
OpenLibraryHandle[OpenLibraryHandleSize++] = h;
}
}
return h;
}
#define PF_DECL(proc) proc##_t pf##proc = NULL

View File

@ -32,7 +32,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
IDD_DIALOG DIALOGEX 12, 12, 206, 329
STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | DS_CENTER | WS_POPUP | WS_CAPTION | WS_SYSMENU
CAPTION "Rufus 1.4.7.460"
CAPTION "Rufus 1.4.7.461"
FONT 8, "MS Shell Dlg", 400, 0, 0x1
BEGIN
DEFPUSHBUTTON "Start",IDC_START,94,291,50,14
@ -165,7 +165,7 @@ END
RTL_IDD_DIALOG DIALOGEX 12, 12, 206, 329
STYLE DS_SETFONT | DS_MODALFRAME | DS_FIXEDSYS | DS_CENTER | WS_POPUP | WS_CAPTION | WS_SYSMENU
EXSTYLE WS_EX_RTLREADING | WS_EX_APPWINDOW | WS_EX_LAYOUTRTL
CAPTION "Rufus 1.4.7.460"
CAPTION "Rufus 1.4.7.461"
FONT 8, "MS Shell Dlg", 400, 0, 0x1
BEGIN
DEFPUSHBUTTON "Start",IDC_START,94,291,50,14
@ -427,8 +427,8 @@ END
//
VS_VERSION_INFO VERSIONINFO
FILEVERSION 1,4,7,460
PRODUCTVERSION 1,4,7,460
FILEVERSION 1,4,7,461
PRODUCTVERSION 1,4,7,461
FILEFLAGSMASK 0x3fL
#ifdef _DEBUG
FILEFLAGS 0x1L
@ -445,13 +445,13 @@ BEGIN
BEGIN
VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)"
VALUE "FileDescription", "Rufus"
VALUE "FileVersion", "1.4.7.460"
VALUE "FileVersion", "1.4.7.461"
VALUE "InternalName", "Rufus"
VALUE "LegalCopyright", "© 2011-2014 Pete Batard (GPL v3)"
VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html"
VALUE "OriginalFilename", "rufus.exe"
VALUE "ProductName", "Rufus"
VALUE "ProductVersion", "1.4.7.460"
VALUE "ProductVersion", "1.4.7.461"
END
END
BLOCK "VarFileInfo"

View File

@ -39,7 +39,7 @@ BOOL is_x64(void)
// Detect if we're running a 32 or 64 bit system
if (sizeof(uintptr_t) < 8) {
pIsWow64Process = (BOOL (__stdcall *)(HANDLE, PBOOL))
GetProcAddress(GetModuleHandleA("KERNEL32"), "IsWow64Process");
GetProcAddress(GetDLLHandle("KERNEL32"), "IsWow64Process");
if (pIsWow64Process != NULL) {
(*pIsWow64Process)(GetCurrentProcess(), &ret);
}

View File

@ -47,14 +47,14 @@ static HRESULT (WINAPI *pSHCreateItemFromParsingName)(PCWSTR, IBindCtx*, REFIID,
#endif
#define INIT_VISTA_SHELL32 if (pSHCreateItemFromParsingName == NULL) { \
pSHCreateItemFromParsingName = (HRESULT (WINAPI *)(PCWSTR, IBindCtx*, REFIID, void **)) \
GetProcAddress(GetModuleHandleA("SHELL32"), "SHCreateItemFromParsingName"); \
GetProcAddress(GetDLLHandle("SHELL32"), "SHCreateItemFromParsingName"); \
}
#define IS_VISTA_SHELL32_AVAILABLE (pSHCreateItemFromParsingName != NULL)
// And this one is simply not available in MinGW32
static LPITEMIDLIST (WINAPI *pSHSimpleIDListFromPath)(PCWSTR pszPath) = NULL;
#define INIT_XP_SHELL32 if (pSHSimpleIDListFromPath == NULL) { \
pSHSimpleIDListFromPath = (LPITEMIDLIST (WINAPI *)(PCWSTR)) \
GetProcAddress(GetModuleHandleA("SHELL32"), "SHSimpleIDListFromPath"); \
GetProcAddress(GetDLLHandle("SHELL32"), "SHSimpleIDListFromPath"); \
}
/* Globals */

View File

@ -192,6 +192,7 @@ static BOOL WimExtractFile_7z(const char* image, int index, const char* src, con
uprintf(" 7z.exe did not extract %s\n", tmpdst);
return FALSE;
}
// coverity[toctou]
if (rename(tmpdst, dst) != 0) {
uprintf(" Could not rename %s to %s\n", tmpdst, dst);
return FALSE;