From f453dc272b93adb0d63dd045197f4661fc653211 Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Mon, 30 Sep 2024 17:38:47 +0100 Subject: [PATCH] [misc] fix a potential double free and avoid nonsensical error messages * buf could be freed twice in iso.c. * Using HRESULT_CODE(error_code) in WindowsErrorString() could lead to "Error: SUCCESS" messages. * Closes #2524. * Also try to address potential issues that appear to have been seen in the wild. --- src/dev.c | 4 ++-- src/drive.c | 6 ++++-- src/hash.c | 7 +++++-- src/iso.c | 2 +- src/rufus.rc | 10 +++++----- src/stdio.c | 6 +++--- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/dev.c b/src/dev.c index 801fb12430e..5ea2b2a06ab 100644 --- a/src/dev.c +++ b/src/dev.c @@ -877,8 +877,8 @@ BOOL GetDevices(DWORD devnum) continue; } - hDrive = CreateFileA(devint_detail_data->DevicePath, GENERIC_READ|GENERIC_WRITE, - FILE_SHARE_READ|FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL); + hDrive = CreateFileWithTimeout(devint_detail_data->DevicePath, GENERIC_READ|GENERIC_WRITE, + FILE_SHARE_READ|FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL, 3000); if(hDrive == INVALID_HANDLE_VALUE) { uprintf("Could not open '%s': %s", devint_detail_data->DevicePath, WindowsErrorString()); continue; diff --git a/src/drive.c b/src/drive.c index 8d0514d8fde..5f378e2424a 100644 --- a/src/drive.c +++ b/src/drive.c @@ -1894,10 +1894,12 @@ BOOL GetDrivePartitionData(DWORD DriveIndex, char* FileSystemName, DWORD FileSys SelectedDrive.SectorsPerTrack = DiskGeometry->Geometry.SectorsPerTrack; SelectedDrive.MediaType = DiskGeometry->Geometry.MediaType; - suprintf("Disk type: %s, Disk size: %s, Sector size: %d bytes", (SelectedDrive.MediaType == FixedMedia)?"FIXED":"Removable", + suprintf("Disk type: %s, Disk size: %s, Sector size: %d bytes", + (SelectedDrive.MediaType == FixedMedia) ? "FIXED" : "Removable", SizeToHumanReadable(SelectedDrive.DiskSize, FALSE, TRUE), SelectedDrive.SectorSize); suprintf("Cylinders: %" PRIi64 ", Tracks per cylinder: %d, Sectors per track: %d", DiskGeometry->Geometry.Cylinders, DiskGeometry->Geometry.TracksPerCylinder, DiskGeometry->Geometry.SectorsPerTrack); + assert(SelectedDrive.SectorSize != 0); r = DeviceIoControl(hPhysical, IOCTL_DISK_GET_DRIVE_LAYOUT_EX, NULL, 0, layout, sizeof(layout), &size, NULL ); if (!r || size <= 0) { @@ -1965,7 +1967,7 @@ BOOL GetDrivePartitionData(DWORD DriveIndex, char* FileSystemName, DWORD FileSys SizeToHumanReadable(DriveLayout->PartitionEntry[i].PartitionLength.QuadPart, TRUE, FALSE), DriveLayout->PartitionEntry[i].PartitionLength.QuadPart, DriveLayout->PartitionEntry[i].StartingOffset.QuadPart / SelectedDrive.SectorSize, - DriveLayout->PartitionEntry[i].Mbr.BootIndicator?"Yes":"No"); + DriveLayout->PartitionEntry[i].Mbr.BootIndicator ? "Yes" : "No"); // suprintf(" GUID: %s", GuidToString(&DriveLayout->PartitionEntry[i].Mbr.PartitionId)); SelectedDrive.FirstDataSector = min(SelectedDrive.FirstDataSector, (DWORD)(DriveLayout->PartitionEntry[i].StartingOffset.QuadPart / SelectedDrive.SectorSize)); diff --git a/src/hash.c b/src/hash.c index a99d297bfa8..76315edc54d 100644 --- a/src/hash.c +++ b/src/hash.c @@ -1607,8 +1607,7 @@ static int cmp_pe_section(const void* arg1, const void* arg2) * @len: Size of @efi * @regp: Pointer to a list of regions * - * Parse image binary in PE32(+) format, assuming that sanity of PE image - * has been checked by a caller. + * Parse image binary in PE32(+) format. * * Return: TRUE on success, FALSE on error */ @@ -1623,7 +1622,11 @@ BOOL efi_image_parse(uint8_t* efi, size_t len, struct efi_image_regions** regp) uint32_t align, size, authsz; size_t bytes_hashed; + if (len < 0x80) + return FALSE; dos = (void*)efi; + if (dos->e_lfanew > len - 0x40) + return FALSE; nt = (void*)(efi + dos->e_lfanew); authsz = 0; diff --git a/src/iso.c b/src/iso.c index a5b978be362..485de74cc38 100644 --- a/src/iso.c +++ b/src/iso.c @@ -1268,7 +1268,7 @@ BOOL ExtractISO(const char* src_iso, const char* dest_dir, BOOL scan) sl_index = i; } } - free(buf); + safe_free(buf); } DeleteFileU(isolinux_tmp); } diff --git a/src/rufus.rc b/src/rufus.rc index e81d2d42c6b..9a2845eb1c0 100644 --- a/src/rufus.rc +++ b/src/rufus.rc @@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL IDD_DIALOG DIALOGEX 12, 12, 232, 326 STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU EXSTYLE WS_EX_ACCEPTFILES -CAPTION "Rufus 4.6.2194" +CAPTION "Rufus 4.6.2195" FONT 9, "Segoe UI Symbol", 400, 0, 0x0 BEGIN LTEXT "Drive Properties",IDS_DRIVE_PROPERTIES_TXT,8,6,53,12,NOT WS_GROUP @@ -397,8 +397,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 4,6,2194,0 - PRODUCTVERSION 4,6,2194,0 + FILEVERSION 4,6,2195,0 + PRODUCTVERSION 4,6,2195,0 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -416,13 +416,13 @@ BEGIN VALUE "Comments", "https://rufus.ie" VALUE "CompanyName", "Akeo Consulting" VALUE "FileDescription", "Rufus" - VALUE "FileVersion", "4.6.2194" + VALUE "FileVersion", "4.6.2195" VALUE "InternalName", "Rufus" VALUE "LegalCopyright", "� 2011-2024 Pete Batard (GPL v3)" VALUE "LegalTrademarks", "https://www.gnu.org/licenses/gpl-3.0.html" VALUE "OriginalFilename", "rufus-4.6.exe" VALUE "ProductName", "Rufus" - VALUE "ProductVersion", "4.6.2194" + VALUE "ProductVersion", "4.6.2195" END END BLOCK "VarFileInfo" diff --git a/src/stdio.c b/src/stdio.c index 59909196c37..4004575189f 100644 --- a/src/stdio.c +++ b/src/stdio.c @@ -260,8 +260,8 @@ const char *WindowsErrorString(void) // coverity[var_deref_model] size = FormatMessageU(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | ((hModule != NULL) ? FORMAT_MESSAGE_FROM_HMODULE : 0), hModule, - HRESULT_CODE(error_code), MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), - &err_string[presize], (DWORD)(sizeof(err_string)-strlen(err_string)), NULL); + error_code, MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), + &err_string[presize], (DWORD)(sizeof(err_string) - strlen(err_string)), NULL); if (size == 0) { format_error = GetLastError(); switch (format_error) { @@ -519,7 +519,7 @@ HANDLE CreateFileWithTimeout(LPCSTR lpFileName, DWORD dwDesiredAccess, DWORD dwS if (hThread != NULL) { if (WaitForSingleObject(hThread, dwTimeOut) == WAIT_TIMEOUT) { CancelSynchronousIo(hThread); - WaitForSingleObject(hThread, INFINITE); + WaitForSingleObject(hThread, 30000); params.dwError = WAIT_TIMEOUT; } CloseHandle(hThread);