Skip to content

Commit e43d6bb

Browse files
committed
[core] fix crash when trying to open a Windows ISO with a very long path
* The current wimlib code makes repeated attempts to close stdin (0) on cleanup when a WIM cannot be opened, which doesn't sit too well with MSFT's _close() as it invokes an invalid parameter exception handler that can make the application crash... * This may happen when we try to open the WIM from an ISO that has been truncated because it resides on a path that is longer than MAX_PATH on account that wimlib repeatedly attempts to close the phantom stdin fd's it sees assigned to its internal (and unused) WIMStruct after it errors out on trying to open the image. * So we declare stdin as invalid for use with Visual Studio compiled apps. * For good measure we also increase the size of the string arrays we used for WIM paths to 1024 UTF-8 characters, and add explicit asserts in case we have to truncate these paths (since we are quite curious about real-life scenarios where people need paths longer than 1024). * Closes pbatard#2777. * Also improve the safe_strcp() and safe_sprintf() macros and fix some unwarranted "Command was terminated by user" messages introduced in commit ea01cd4.
1 parent c937930 commit e43d6bb

7 files changed

Lines changed: 39 additions & 17 deletions

File tree

src/iso.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ static BOOL check_iso_props(const char* psz_dirname, int64_t file_length, const
246246
const char* psz_fullpath, EXTRACT_PROPS *props)
247247
{
248248
size_t i, j, k, len;
249-
char bootloader_name[32], path[MAX_PATH];
249+
char bootloader_name[32];
250250

251251
// Check for an isolinux/syslinux config file anywhere
252252
memset(props, 0, sizeof(EXTRACT_PROPS));
@@ -298,15 +298,17 @@ static BOOL check_iso_props(const char* psz_dirname, int64_t file_length, const
298298
if (file_length >= 4 * GB && psz_dirname != NULL && IS_FAT(fs_type) && img_report.has_4GB_file == 0x81) {
299299
if (safe_stricmp(&psz_dirname[max(0, ((int)safe_strlen(psz_dirname)) -
300300
((int)strlen(sources_str)))], sources_str) == 0) {
301+
char wim_path[4 * MAX_PATH];
301302
for (i = 0; i < ARRAYSIZE(wininst_name) - 1; i++) {
302303
if (safe_stricmp(psz_basename, wininst_name[i]) == 0 && file_length >= 4 * GB) {
303304
print_split_file((char*)psz_fullpath, file_length);
304305
char* dst = safe_strdup(psz_fullpath);
305306
dst[strlen(dst) - 3] = 's';
306307
dst[strlen(dst) - 2] = 'w';
307308
dst[strlen(dst) - 1] = 'm';
308-
static_sprintf(path, "%s|%s/%s", image_path, psz_dirname, psz_basename);
309-
WimSplitFile(path, dst);
309+
assert(safe_strlen(image_path) + safe_strlen(psz_dirname) + safe_strlen(psz_basename) + 2 < ARRAYSIZE(wim_path));
310+
static_sprintf(wim_path, "%s|%s/%s", image_path, psz_dirname, psz_basename);
311+
WimSplitFile(wim_path, dst);
310312
free(dst);
311313
return TRUE;
312314
}
@@ -1401,8 +1403,10 @@ BOOL ExtractISO(const char* src_iso, const char* dest_dir, BOOL scan)
14011403
safe_free(buf);
14021404
}
14031405
if (HAS_WININST(img_report)) {
1404-
static_sprintf(path, "%s|%s", image_path, &img_report.wininst_path[0][2]);
1405-
img_report.wininst_version = GetWimVersion(path);
1406+
char wim_path[4 * MAX_PATH];
1407+
assert(safe_strlen(image_path) + safe_strlen(&img_report.wininst_path[0][2]) + 2 < ARRAYSIZE(wim_path));
1408+
static_sprintf(wim_path, "%s|%s", image_path, &img_report.wininst_path[0][2]);
1409+
img_report.wininst_version = GetWimVersion(wim_path);
14061410
}
14071411
if (img_report.has_grub2) {
14081412
char grub_path[128];

src/msapi_utf8.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,12 @@ extern "C" {
5656

5757
#define wchar_to_utf8_no_alloc(wsrc, dest, dest_size) \
5858
WideCharToMultiByte(CP_UTF8, 0, wsrc, -1, dest, (int)(dest_size), NULL, NULL)
59+
#define wchar_to_utf8_get_size(wsrc) \
60+
WideCharToMultiByte(CP_UTF8, 0, wsrc, -1, NULL, 0, NULL, NULL)
5961
#define utf8_to_wchar_no_alloc(src, wdest, wdest_size) \
6062
MultiByteToWideChar(CP_UTF8, 0, src, -1, wdest, (int)(wdest_size))
63+
#define utf8_to_wchar_get_size(src) \
64+
MultiByteToWideChar(CP_UTF8, 0, src, -1, NULL, 0)
6165
#define Edit_ReplaceSelU(hCtrl, str) ((void)SendMessageLU(hCtrl, EM_REPLACESEL, (WPARAM)FALSE, str))
6266
#define ComboBox_AddStringU(hCtrl, str) ((int)(DWORD)SendMessageLU(hCtrl, CB_ADDSTRING, (WPARAM)FALSE, str))
6367
#define ComboBox_InsertStringU(hCtrl, index, str) ((int)(DWORD)SendMessageLU(hCtrl, CB_INSERTSTRING, (WPARAM)index, str))

src/rufus.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@
168168
#define safe_mm_free(p) do { _mm_free((void*)p); p = NULL; } while(0)
169169
static __inline void safe_strcp(char* dst, const size_t dst_max, const char* src, const size_t count) {
170170
memmove(dst, src, min(count, dst_max));
171-
dst[min(count, dst_max) - 1] = 0;
171+
if (dst != NULL) dst[min(count, dst_max) - 1] = 0;
172172
}
173173
#define safe_strcpy(dst, dst_max, src) safe_strcp(dst, dst_max, src, safe_strlen(src) + 1)
174174
#define static_strcpy(dst, src) safe_strcpy(dst, sizeof(dst), src)
@@ -188,7 +188,7 @@ static __inline void safe_strcp(char* dst, const size_t dst_max, const char* src
188188
HIMAGELIST _hImageList = (HIMAGELIST)SendMessage(hToolbar, TB_GETIMAGELIST, (WPARAM)0, (LPARAM)0); \
189189
safe_destroy_imagelist(_hImageList); } } while(0)
190190
#define safe_sprintf(dst, count, ...) do { size_t _count = count; char* _dst = dst; _snprintf_s(_dst, _count, _TRUNCATE, __VA_ARGS__); \
191-
_dst[(_count) - 1] = 0; } while(0)
191+
if (_dst != NULL) _dst[(_count) - 1] = 0; } while(0)
192192
#define static_sprintf(dst, ...) safe_sprintf(dst, sizeof(dst), __VA_ARGS__)
193193
#define safe_atoi(str) ((((char*)(str))==NULL) ? 0 : atoi(str))
194194
#define safe_strlen(str) ((((char*)(str))==NULL) ? 0 : strlen(str))

src/rufus.rc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
3333
IDD_DIALOG DIALOGEX 12, 12, 232, 326
3434
STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU
3535
EXSTYLE WS_EX_ACCEPTFILES
36-
CAPTION "Rufus 4.10.2268"
36+
CAPTION "Rufus 4.10.2269"
3737
FONT 9, "Segoe UI Symbol", 400, 0, 0x0
3838
BEGIN
3939
LTEXT "Drive Properties",IDS_DRIVE_PROPERTIES_TXT,8,6,53,12,NOT WS_GROUP
@@ -407,8 +407,8 @@ END
407407
//
408408

409409
VS_VERSION_INFO VERSIONINFO
410-
FILEVERSION 4,10,2268,0
411-
PRODUCTVERSION 4,10,2268,0
410+
FILEVERSION 4,10,2269,0
411+
PRODUCTVERSION 4,10,2269,0
412412
FILEFLAGSMASK 0x3fL
413413
#ifdef _DEBUG
414414
FILEFLAGS 0x1L
@@ -426,13 +426,13 @@ BEGIN
426426
VALUE "Comments", "https://rufus.ie"
427427
VALUE "CompanyName", "Akeo Consulting"
428428
VALUE "FileDescription", "Rufus"
429-
VALUE "FileVersion", "4.10.2268"
429+
VALUE "FileVersion", "4.10.2269"
430430
VALUE "InternalName", "Rufus"
431431
VALUE "LegalCopyright", "� 2011-2025 Pete Batard (GPL v3)"
432432
VALUE "LegalTrademarks", "https://www.gnu.org/licenses/gpl-3.0.html"
433433
VALUE "OriginalFilename", "rufus-4.10.exe"
434434
VALUE "ProductName", "Rufus"
435-
VALUE "ProductVersion", "4.10.2268"
435+
VALUE "ProductVersion", "4.10.2269"
436436
END
437437
END
438438
BLOCK "VarFileInfo"

src/stdfn.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -890,13 +890,13 @@ DWORD RunCommandWithProgress(const char* cmd, const char* dir, BOOL log, int msg
890890
Sleep(100);
891891
};
892892
} else {
893-
// TODO: Detect user cancellation here?
894893
switch (WaitForSingleObject(pi.hProcess, 1800000)) {
895894
case WAIT_TIMEOUT:
896895
uprintf("Command did not terminate within timeout duration");
897896
break;
898897
case WAIT_OBJECT_0:
899-
uprintf("Command was terminated by user");
898+
if (IS_ERROR(ErrorStatus) && (SCODE_CODE(ErrorStatus) == ERROR_CANCELLED))
899+
uprintf("Command was terminated by user");
900900
break;
901901
default:
902902
uprintf("Error while waiting for command to be terminated: %s", WindowsErrorString());

src/wimlib/wimlib/file_io.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,15 @@ static inline void filedes_invalidate(struct filedes *fd)
8181
static inline bool
8282
filedes_valid(const struct filedes *fd)
8383
{
84+
#ifdef _MSC_VER
85+
// The current wimlib code makes repeated attempts to close stdin (0) when a
86+
// WIM cannot be opened, which doesn't sit too well with MSFT's _close() as
87+
// it invokes the exception handler and can make the application crash...
88+
// So we declare stdin as invalid for use with Visual Studio compiled apps.
89+
return (fd->fd != -1 && fd->fd != 0);
90+
#else
8491
return fd->fd != -1;
92+
#endif
8593
}
8694

8795
#endif /* _WIMLIB_FILE_IO_H */

src/wue.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,15 +475,17 @@ static void PopulateWindowsVersionFromXml(const wchar_t* xml, size_t xml_len, in
475475
BOOL PopulateWindowsVersion(void)
476476
{
477477
int r;
478-
char wim_path[MAX_PATH] = "";
478+
char wim_path[4 * MAX_PATH] = "";
479479
wchar_t* xml = NULL;
480480
size_t xml_len;
481481
WIMStruct* wim = NULL;
482482

483483
memset(&img_report.win_version, 0, sizeof(img_report.win_version));
484484

485+
assert(safe_strlen(image_path) + 1 < ARRAYSIZE(wim_path));
485486
static_strcpy(wim_path, image_path);
486487
if (!img_report.is_windows_img) {
488+
assert(safe_strlen(image_path) + safe_strlen(&img_report.wininst_path[0][3]) + 1 < ARRAYSIZE(wim_path));
487489
static_strcat(wim_path, "|");
488490
static_strcat(wim_path, &img_report.wininst_path[0][3]);
489491
}
@@ -545,7 +547,7 @@ int SetWinToGoIndex(void)
545547
int i, r;
546548
WIMStruct* wim = NULL;
547549
char* install_names[MAX_WININST];
548-
wchar_t wim_path[MAX_PATH] = L"", *xml = NULL;
550+
wchar_t wim_path[4 * MAX_PATH] = L"", *xml = NULL;
549551
size_t xml_len;
550552
StrArray version_name = { 0 }, version_index = { 0 };
551553
BOOL bNonStandard = FALSE;
@@ -568,9 +570,11 @@ int SetWinToGoIndex(void)
568570
wininst_index = 0;
569571
}
570572

573+
assert(utf8_to_wchar_get_size(image_path) <= ARRAYSIZE(wim_path));
571574
utf8_to_wchar_no_alloc(image_path, wim_path, ARRAYSIZE(wim_path));
572575
if (!img_report.is_windows_img) {
573576
wcscat(wim_path, L"|");
577+
assert(utf8_to_wchar_get_size(image_path) + utf8_to_wchar_get_size(&img_report.wininst_path[wininst_index][2]) <= ARRAYSIZE(wim_path));
574578
utf8_to_wchar_no_alloc(&img_report.wininst_path[wininst_index][2],
575579
&wim_path[wcslen(wim_path)], (int)ARRAYSIZE(wim_path) - wcslen(wim_path));
576580
}
@@ -663,7 +667,7 @@ int SetWinToGoIndex(void)
663667
/// <returns>TRUE on success, FALSE on error.</returns>
664668
BOOL SetupWinToGo(DWORD DriveIndex, const char* drive_name, BOOL use_esp)
665669
{
666-
char *ms_efi = NULL, wim_path[MAX_PATH], cmd[MAX_PATH];
670+
char *ms_efi = NULL, wim_path[4 * MAX_PATH], cmd[MAX_PATH];
667671
ULONG cluster_size;
668672

669673
uprintf("Windows To Go mode selected");
@@ -673,8 +677,10 @@ BOOL SetupWinToGo(DWORD DriveIndex, const char* drive_name, BOOL use_esp)
673677
return FALSE;
674678
}
675679

680+
assert(safe_strlen(image_path) < ARRAYSIZE(wim_path));
676681
static_strcpy(wim_path, image_path);
677682
if (!img_report.is_windows_img) {
683+
assert(safe_strlen(image_path) + safe_strlen(&img_report.wininst_path[wininst_index][3]) + 1 < ARRAYSIZE(wim_path));
678684
static_strcat(wim_path, "|");
679685
static_strcat(wim_path, &img_report.wininst_path[wininst_index][3]);
680686
}

0 commit comments

Comments
 (0)