[iso] fix a buffer overflow in syslinux.c

* p[safe_strlen(p)] = 0; was pointless and could lead to a buffer overflow if
  the string was not already NUL terminated, so remove it and make sure we
  process a buffer that either contains legitimate Syslinux version strings
  (that are NUL terminated always) or that has been read through read_file()
  (that always adds a NUL terminator to the buffer).
* Also fix some whitespaces in related code sections and switch to using
  read_file() for GRUB version lookup.
* Vulnerability discovered and reported by Mansour Gashasbi (@gashasbi).
This commit is contained in:
Pete Batard 2024-04-10 10:26:31 +02:00
parent 34e6e43a97
commit f813eb05d8
No known key found for this signature in database
GPG Key ID: 38E0CF5E69EDD671
4 changed files with 22 additions and 34 deletions

View File

@ -1233,18 +1233,9 @@ out:
char isolinux_tmp[MAX_PATH];
static_sprintf(isolinux_tmp, "%sisolinux.tmp", temp_dir);
size = (size_t)ExtractISOFile(src_iso, isolinux_path.String[i], isolinux_tmp, FILE_ATTRIBUTE_NORMAL);
if (size == 0) {
if ((size == 0) || (read_file(isolinux_tmp, &buf) != size)) {
uprintf(" Could not access %s", isolinux_path.String[i]);
} else {
buf = (char*)calloc(size, 1);
if (buf == NULL) break;
fd = fopen(isolinux_tmp, "rb");
if (fd == NULL) {
free(buf);
continue;
}
fread(buf, 1, size, fd);
fclose(fd);
sl_version = GetSyslinuxVersion(buf, size, &ext);
if (img_report.sl_version == 0) {
static_strcpy(img_report.sl_version_ext, ext);
@ -1315,15 +1306,10 @@ out:
// coverity[swapped_arguments]
if (GetTempFileNameU(temp_dir, APPLICATION_NAME, 0, path) != 0) {
size = (size_t)ExtractISOFile(src_iso, grub_path, path, FILE_ATTRIBUTE_NORMAL);
buf = (char*)calloc(size, 1);
fd = fopen(path, "rb");
if ((size == 0) || (buf == NULL) || (fd == NULL)) {
if ((size == 0) || (read_file(path, &buf) != size))
uprintf(" Could not read Grub version from '%s'", grub_path);
} else {
fread(buf, 1, size, fd);
fclose(fd);
else
GetGrubVersion(buf, size);
}
free(buf);
DeleteFileU(path);
}

View File

@ -2032,7 +2032,7 @@ static void InitDialog(HWND hDlg)
len = 0;
buf = (char*)GetResource(hMainInstance, resource[i], _RT_RCDATA, "ldlinux_sys", &len, TRUE);
if (buf == NULL) {
uprintf("Warning: could not read embedded Syslinux v%d version", i+4);
uprintf("Warning: could not read embedded Syslinux v%d version", i + 4);
} else {
embedded_sl_version[i] = GetSyslinuxVersion(buf, len, &ext);
static_sprintf(embedded_sl_version_str[i], "%d.%02d", SL_MAJOR(embedded_sl_version[i]), SL_MINOR(embedded_sl_version[i]));

View File

@ -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.5.2127"
CAPTION "Rufus 4.5.2128"
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,5,2127,0
PRODUCTVERSION 4,5,2127,0
FILEVERSION 4,5,2128,0
PRODUCTVERSION 4,5,2128,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.5.2127"
VALUE "FileVersion", "4.5.2128"
VALUE "InternalName", "Rufus"
VALUE "LegalCopyright", "<22> 2011-2024 Pete Batard (GPL v3)"
VALUE "LegalTrademarks", "https://www.gnu.org/licenses/gpl-3.0.html"
VALUE "OriginalFilename", "rufus-4.5.exe"
VALUE "ProductName", "Rufus"
VALUE "ProductVersion", "4.5.2127"
VALUE "ProductVersion", "4.5.2128"
END
END
BLOCK "VarFileInfo"

View File

@ -411,28 +411,30 @@ uint16_t GetSyslinuxVersion(char* buf, size_t buf_size, char** ext)
return 0;
// Start at 64 to avoid the short incomplete version at the beginning of ldlinux.sys
for (i=64; i<buf_size-64; i++) {
for (i = 64; i < buf_size - 64; i++) {
if (memcmp(&buf[i], LINUX, sizeof(LINUX)) == 0) {
// Check for ISO or SYS prefix
if (!( ((buf[i-3] == 'I') && (buf[i-2] == 'S') && (buf[i-1] == 'O'))
|| ((buf[i-3] == 'S') && (buf[i-2] == 'Y') && (buf[i-1] == 'S')) ))
if (!( ((buf[i - 3] == 'I') && (buf[i - 2] == 'S') && (buf[i - 1] == 'O'))
|| ((buf[i - 3] == 'S') && (buf[i - 2] == 'Y') && (buf[i - 1] == 'S')) ))
continue;
i += sizeof(LINUX);
version = (((uint8_t)strtoul(&buf[i], &p, 10))<<8) + (uint8_t)strtoul(&p[1], &p, 10);
version = (((uint8_t)strtoul(&buf[i], &p, 10)) << 8) + (uint8_t)strtoul(&p[1], &p, 10);
// Our buffer is either from our internal legit syslinux (i.e. with a NUL terminated
// version string) or from a buffer that has been NUL-terminated through read_file(),
// so the string we work with in p is always NUL terminated at this stage.
if (version == 0)
continue;
p[safe_strlen(p)] = 0;
// Ensure that our extra version string starts with a slash
*p = '/';
// Remove the x.yz- duplicate if present
for (j=0; (buf[i+j] == p[1+j]) && (buf[i+j] != ' '); j++);
if (p[j+1] == '-')
for (j = 0; (buf[i + j] == p[1 + j]) && (buf[i + j] != ' '); j++);
if (p[j + 1] == '-')
j++;
if (j >= 4) {
p[j] = '/';
p = &p[j];
}
for (j=safe_strlen(p)-1; j>0; j--) {
for (j = safe_strlen(p) - 1; j > 0; j--) {
// Arch Linux affixes a star for their version - who knows what else is out there...
if ((p[j] == ' ') || (p[j] == '*'))
p[j] = 0;
@ -440,15 +442,15 @@ uint16_t GetSyslinuxVersion(char* buf, size_t buf_size, char** ext)
break;
}
// Sanitize the string
for (j=1; j<safe_strlen(p); j++) {
for (j = 1; j < safe_strlen(p); j++) {
// Some people are bound to have invalid chars in their date strings
for (k=0; k<sizeof(unauthorized); k++) {
for (k = 0; k < sizeof(unauthorized); k++) {
if (p[j] == unauthorized[k])
p[j] = '_';
}
}
// If all we have is a slash, return the empty string for the extra version
*ext = (p[1] == 0)?nullstr:p;
*ext = (p[1] == 0) ? nullstr : p;
return version;
}
}