From 38b8a8feec4c48d7d4aa6731955b2a96c0d2ec7d Mon Sep 17 00:00:00 2001 From: Michael Chang Date: Thu, 16 Apr 2026 11:11:10 +0800 Subject: [PATCH] Fix problematic utf8 conversion in bli patches Use UTF-16 code unit counts in grub-core/commands/bli.c when converting EFI variable data to UTF-8. Allocate the UTF-8 buffer with checked size growth, convert using the correct UTF-16 element count, and always append a terminating NUL. This avoids heap overreads, heap overflows, and unterminated strings when EFI variable contents are malformed or larger than expected. Fix filename growth handling in grub-core/commands/bls_bumpcounter.c by accounting for the extra UTF-16 storage needed when appending "-1" to the filename. Use checked size expansion before reallocating the EFI file info buffer, and pass the UTF-16 destination length in code units to grub_utf8_to_utf16(). This prevents writing past FileName when the renamed entry becomes longer. Bound the scan of LoaderEntries in grub-core/commands/blsuki.c by the EFI variable size instead of assuming that the data is always NUL-terminated. If no UTF-16 terminator is found within the reported variable size, treat the variable as malformed and fail cleanly. This avoids out-of-bounds reads on corrupted or malicious EFI variable contents. Signed-off-by: Michael Chang --- grub-core/commands/bli.c | 26 +++++++++++++++++++++----- grub-core/commands/bls_bumpcounter.c | 13 +++++++++---- grub-core/commands/blsuki.c | 24 +++++++++++++++++++++--- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/grub-core/commands/bli.c b/grub-core/commands/bli.c index 72a3a4edc..c90c9b404 100644 --- a/grub-core/commands/bli.c +++ b/grub-core/commands/bli.c @@ -177,19 +177,35 @@ static char* get_efivar (const char* efivar) { grub_efi_status_t status; - grub_size_t size; + grub_size_t size, size16; grub_efi_char16_t *val_efi = NULL; char *val = NULL; + status = grub_efi_get_variable (efivar, &bli_vendor_guid, &size, (void**) &val_efi); - if (status == GRUB_EFI_SUCCESS && size != 0) + + size16 = size / sizeof (*val_efi); + if (status == GRUB_EFI_SUCCESS && size16 > 0) { - val = grub_malloc (size * sizeof (char)); - grub_utf16_to_utf8 ((grub_uint8_t*) val, - (grub_uint16_t*) val_efi, size); + grub_size_t size_utf8; + + if (grub_mul (size16, GRUB_MAX_UTF8_PER_UTF16, &size_utf8) || + grub_add (size_utf8, 1, &size_utf8)) + { + grub_error (GRUB_ERR_OUT_OF_RANGE, "size overflow when converting EFI variable %s to UTF-8", efivar); + goto finish; + } + val = grub_malloc (size_utf8); + if (val == NULL) + goto finish; + *grub_utf16_to_utf8 ((grub_uint8_t*) val, + (grub_uint16_t*) val_efi, size16) = '\0'; } + +finish: + grub_free (val_efi); return val; } diff --git a/grub-core/commands/bls_bumpcounter.c b/grub-core/commands/bls_bumpcounter.c index 014389567..d45fe276c 100644 --- a/grub-core/commands/bls_bumpcounter.c +++ b/grub-core/commands/bls_bumpcounter.c @@ -192,9 +192,13 @@ grub_cmd_bls_bumpcounter (grub_extcmd_context_t ctxt __attribute__ ((unused)), { grub_free(file_info); /* When we bump the counter, it might happen that we increase the filename - length. Add the space for 2 more characters. (i.e. adding the boot done - part "-1") */ - size += 2; + length. Add the space for 2 more characters (4 bytes). (i.e. adding the + boot done part "-1") */ + if (grub_add (size, 4, &size)) + { + grub_error (GRUB_ERR_OUT_OF_RANGE, N_("size overflow")); + goto finish; + } file_info = grub_malloc (size); if (!file_info) { @@ -218,7 +222,8 @@ grub_cmd_bls_bumpcounter (grub_extcmd_context_t ctxt __attribute__ ((unused)), grub_dprintf ("bls_bumpcounter", "renaming entry to %s\n", new_path); grub_utf8_to_utf16 (file_info->FileName, - size - offsetof (grub_efi_file_info_t, FileName), + (size - offsetof (grub_efi_file_info_t, FileName)) + / sizeof (grub_efi_char16_t), (grub_uint8_t*) new_path, grub_strlen (new_path), NULL); err = handle->SetInfo (handle, &grub_efi_file_info_guid, size, file_info); diff --git a/grub-core/commands/blsuki.c b/grub-core/commands/blsuki.c index e692bbe69..4dfc4934e 100644 --- a/grub-core/commands/blsuki.c +++ b/grub-core/commands/blsuki.c @@ -1539,7 +1539,7 @@ set_bli_loader_entries (enum blsuki_cmd_type cmd_type) grub_size_t size = 0, remaining_size, len, written, prev_entries_size = 0, len16; grub_efi_char16_t *prev_entries_start = NULL; grub_size_t prev_entries_start_size = 0; - grub_efi_char16_t *efi_entries = NULL, *prev_entries = NULL, *p = NULL, *tmp16; + grub_efi_char16_t *efi_entries = NULL, *prev_entries = NULL, *p = NULL; char *boot_counting_idx, *prev_entry; const char* env = NULL; grub_efi_status_t status; @@ -1624,12 +1624,30 @@ set_bli_loader_entries (enum blsuki_cmd_type cmd_type) { do { grub_size_t prev_entry_size; + grub_size_t prev_entries_size16 = prev_entries_size / sizeof (*prev_entries); - tmp16 = prev_entries; len16 = 0; - while (*tmp16++ != '\0') + /* + * LoaderEntries comes from EFI variable storage, so it is only + * trusted up to prev_entries_size16 UTF-16 code units. Bound + * the scan in case a malformed variable is missing a + * terminator. + */ + while (len16 < prev_entries_size16 && prev_entries[len16] != '\0') len16++; + /* + * If we reached the end of the buffer without finding a null + * terminator, the variable is malformed and we should bail + * instead of risking an out-of-bounds read. + */ + if (len16 == prev_entries_size16) + { + grub_free (prev_entries_start); + grub_free (efi_entries); + return grub_error (GRUB_ERR_BAD_ARGUMENT, "malformed LoaderEntries variable"); + } + if (grub_mul (len16, GRUB_MAX_UTF8_PER_UTF16, &prev_entry_size) || grub_add (prev_entry_size, 1, &prev_entry_size)) { -- 2.53.0