From d10d6fb1870c09bebb2a39e644c0edcbc26999b3 Mon Sep 17 00:00:00 2001 From: Li Zhou Date: Tue, 30 May 2023 16:49:58 +0800 Subject: [PATCH] grub2/grub-efi: fix CVE-2022-2601/CVE-2022-3775 Porting patches from grub2_2.06-3~deb11u4 to fix CVE-2022-2601/CVE-2022-3775. The source code of grub2_2.06-3~deb11u4 is from: https://snapshot.debian.org/archive/debian/20221124T030451Z/ pool/main/g/grub2/grub2_2.06-3~deb11u4.debian.tar.xz Refer to above source code and this link for the fix: https://lists.gnu.org/archive/html/grub-devel/2022-11/msg00059.html The 1st patch in the list is for making proper context for the 14 patches of the 2 CVEs. No content changes for all the patches from debian release. We do this because grub2/grub-efi is ported from wrlinux for secure boot bringing up. Test plan: - PASS: build grub2/grub-efi. - PASS: build-image and install and boot up on lab/qemu. - PASS: check that the "stx.N" version number is right for both bios(grub2 ver) and uefi(grub-efi ver) boot. Closes-bug: 2020730 Signed-off-by: Li Zhou Change-Id: Ia6c58a2021a786ef92f760b3cfe035fbccedacf7 --- ...ct-non-kernel-files-in-the-shim_lock.patch | 99 ++++++++++++++++ ...d-artificial-limit-to-image-dimensio.patch | 109 +++++++++++++++++ ...hs-exceeds-font-max_glyph_width-or-f.patch | 33 ++++++ ...erflow-in-grub_font_get_glyph_intern.patch | 110 ++++++++++++++++++ ...-integer-overflows-in-grub_font_cons.patch | 81 +++++++++++++ ...0025-font-Remove-grub_font_dup_glyph.patch | 42 +++++++ ...nteger-overflow-in-ensure_comb_space.patch | 48 ++++++++ ...nt-Fix-integer-overflow-in-BMP-index.patch | 65 +++++++++++ ...-underflow-in-binary-search-of-char-.patch | 86 ++++++++++++++ ...b-Enforce-verification-of-font-files.patch | 54 +++++++++ .../0030-fbutil-Fix-integer-overflow.patch | 85 ++++++++++++++ ...ix-an-integer-underflow-in-blit_comb.patch | 91 +++++++++++++++ ..._font_blit_glyph-and-grub_font_blit_.patch | 75 ++++++++++++ ...l_font-to-glyphs-in-ascii_font_glyph.patch | 36 ++++++ ...ix-an-integer-overflow-in-grub_unico.patch | 55 +++++++++ grub/grub-efi/debian/patches/series | 15 +++ ...ct-non-kernel-files-in-the-shim_lock.patch | 99 ++++++++++++++++ ...d-artificial-limit-to-image-dimensio.patch | 109 +++++++++++++++++ ...hs-exceeds-font-max_glyph_width-or-f.patch | 33 ++++++ ...erflow-in-grub_font_get_glyph_intern.patch | 110 ++++++++++++++++++ ...-integer-overflows-in-grub_font_cons.patch | 81 +++++++++++++ ...0008-font-Remove-grub_font_dup_glyph.patch | 42 +++++++ ...nteger-overflow-in-ensure_comb_space.patch | 48 ++++++++ ...nt-Fix-integer-overflow-in-BMP-index.patch | 65 +++++++++++ ...-underflow-in-binary-search-of-char-.patch | 86 ++++++++++++++ ...b-Enforce-verification-of-font-files.patch | 54 +++++++++ .../0013-fbutil-Fix-integer-overflow.patch | 85 ++++++++++++++ ...ix-an-integer-underflow-in-blit_comb.patch | 91 +++++++++++++++ ..._font_blit_glyph-and-grub_font_blit_.patch | 75 ++++++++++++ ...l_font-to-glyphs-in-ascii_font_glyph.patch | 36 ++++++ ...ix-an-integer-overflow-in-grub_unico.patch | 55 +++++++++ grub/grub2/debian/patches/series | 15 +++ 32 files changed, 2168 insertions(+) create mode 100644 grub/grub-efi/debian/patches/0020-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch create mode 100644 grub/grub-efi/debian/patches/0021-video-readers-Add-artificial-limit-to-image-dimensio.patch create mode 100644 grub/grub-efi/debian/patches/0022-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch create mode 100644 grub/grub-efi/debian/patches/0023-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch create mode 100644 grub/grub-efi/debian/patches/0024-font-Fix-several-integer-overflows-in-grub_font_cons.patch create mode 100644 grub/grub-efi/debian/patches/0025-font-Remove-grub_font_dup_glyph.patch create mode 100644 grub/grub-efi/debian/patches/0026-font-Fix-integer-overflow-in-ensure_comb_space.patch create mode 100644 grub/grub-efi/debian/patches/0027-font-Fix-integer-overflow-in-BMP-index.patch create mode 100644 grub/grub-efi/debian/patches/0028-font-Fix-integer-underflow-in-binary-search-of-char-.patch create mode 100644 grub/grub-efi/debian/patches/0029-kern-efi-sb-Enforce-verification-of-font-files.patch create mode 100644 grub/grub-efi/debian/patches/0030-fbutil-Fix-integer-overflow.patch create mode 100644 grub/grub-efi/debian/patches/0031-font-Fix-an-integer-underflow-in-blit_comb.patch create mode 100644 grub/grub-efi/debian/patches/0032-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch create mode 100644 grub/grub-efi/debian/patches/0033-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch create mode 100644 grub/grub-efi/debian/patches/0034-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch create mode 100644 grub/grub2/debian/patches/0003-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch create mode 100644 grub/grub2/debian/patches/0004-video-readers-Add-artificial-limit-to-image-dimensio.patch create mode 100644 grub/grub2/debian/patches/0005-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch create mode 100644 grub/grub2/debian/patches/0006-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch create mode 100644 grub/grub2/debian/patches/0007-font-Fix-several-integer-overflows-in-grub_font_cons.patch create mode 100644 grub/grub2/debian/patches/0008-font-Remove-grub_font_dup_glyph.patch create mode 100644 grub/grub2/debian/patches/0009-font-Fix-integer-overflow-in-ensure_comb_space.patch create mode 100644 grub/grub2/debian/patches/0010-font-Fix-integer-overflow-in-BMP-index.patch create mode 100644 grub/grub2/debian/patches/0011-font-Fix-integer-underflow-in-binary-search-of-char-.patch create mode 100644 grub/grub2/debian/patches/0012-kern-efi-sb-Enforce-verification-of-font-files.patch create mode 100644 grub/grub2/debian/patches/0013-fbutil-Fix-integer-overflow.patch create mode 100644 grub/grub2/debian/patches/0014-font-Fix-an-integer-underflow-in-blit_comb.patch create mode 100644 grub/grub2/debian/patches/0015-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch create mode 100644 grub/grub2/debian/patches/0016-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch create mode 100644 grub/grub2/debian/patches/0017-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch diff --git a/grub/grub-efi/debian/patches/0020-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch b/grub/grub-efi/debian/patches/0020-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch new file mode 100644 index 000000000..18ecc2585 --- /dev/null +++ b/grub/grub-efi/debian/patches/0020-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch @@ -0,0 +1,99 @@ +From 24e6d59ac676791507ff5267bf3bef6cbaff6aef Mon Sep 17 00:00:00 2001 +From: Julian Andres Klode +Date: Thu, 2 Dec 2021 15:03:53 +0100 +Subject: kern/efi/sb: Reject non-kernel files in the shim_lock verifier + +We must not allow other verifiers to pass things like the GRUB modules. +Instead of maintaining a blocklist, maintain an allowlist of things +that we do not care about. + +This allowlist really should be made reusable, and shared by the +lockdown verifier, but this is the minimal patch addressing +security concerns where the TPM verifier was able to mark modules +as verified (or the OpenPGP verifier for that matter), when it +should not do so on shim-powered secure boot systems. + +Fixes: CVE-2022-28735 + +Signed-off-by: Julian Andres Klode +Reviewed-by: Daniel Kiper +--- + grub-core/kern/efi/sb.c | 39 ++++++++++++++++++++++++++++++++++++--- + include/grub/verify.h | 1 + + 2 files changed, 37 insertions(+), 3 deletions(-) + +diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c +index c52ec6226..89c4bb3fd 100644 +--- a/grub-core/kern/efi/sb.c ++++ b/grub-core/kern/efi/sb.c +@@ -119,10 +119,11 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), + void **context __attribute__ ((unused)), + enum grub_verify_flags *flags) + { +- *flags = GRUB_VERIFY_FLAGS_SKIP_VERIFICATION; ++ *flags = GRUB_VERIFY_FLAGS_NONE; + + switch (type & GRUB_FILE_TYPE_MASK) + { ++ /* Files we check. */ + case GRUB_FILE_TYPE_LINUX_KERNEL: + case GRUB_FILE_TYPE_MULTIBOOT_KERNEL: + case GRUB_FILE_TYPE_BSD_KERNEL: +@@ -130,11 +131,43 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), + case GRUB_FILE_TYPE_PLAN9_KERNEL: + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: + *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK; ++ return GRUB_ERR_NONE; + +- /* Fall through. */ ++ /* Files that do not affect secureboot state. */ ++ case GRUB_FILE_TYPE_NONE: ++ case GRUB_FILE_TYPE_LOOPBACK: ++ case GRUB_FILE_TYPE_LINUX_INITRD: ++ case GRUB_FILE_TYPE_OPENBSD_RAMDISK: ++ case GRUB_FILE_TYPE_XNU_RAMDISK: ++ case GRUB_FILE_TYPE_SIGNATURE: ++ case GRUB_FILE_TYPE_PUBLIC_KEY: ++ case GRUB_FILE_TYPE_PUBLIC_KEY_TRUST: ++ case GRUB_FILE_TYPE_PRINT_BLOCKLIST: ++ case GRUB_FILE_TYPE_TESTLOAD: ++ case GRUB_FILE_TYPE_GET_SIZE: ++ case GRUB_FILE_TYPE_FONT: ++ case GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY: ++ case GRUB_FILE_TYPE_CAT: ++ case GRUB_FILE_TYPE_HEXCAT: ++ case GRUB_FILE_TYPE_CMP: ++ case GRUB_FILE_TYPE_HASHLIST: ++ case GRUB_FILE_TYPE_TO_HASH: ++ case GRUB_FILE_TYPE_KEYBOARD_LAYOUT: ++ case GRUB_FILE_TYPE_PIXMAP: ++ case GRUB_FILE_TYPE_GRUB_MODULE_LIST: ++ case GRUB_FILE_TYPE_CONFIG: ++ case GRUB_FILE_TYPE_THEME: ++ case GRUB_FILE_TYPE_GETTEXT_CATALOG: ++ case GRUB_FILE_TYPE_FS_SEARCH: ++ case GRUB_FILE_TYPE_LOADENV: ++ case GRUB_FILE_TYPE_SAVEENV: ++ case GRUB_FILE_TYPE_VERIFY_SIGNATURE: ++ *flags = GRUB_VERIFY_FLAGS_SKIP_VERIFICATION; ++ return GRUB_ERR_NONE; + ++ /* Other files. */ + default: +- return GRUB_ERR_NONE; ++ return grub_error (GRUB_ERR_ACCESS_DENIED, N_("prohibited by secure boot policy")); + } + } + +diff --git a/include/grub/verify.h b/include/grub/verify.h +index 6fde244fc..67448165f 100644 +--- a/include/grub/verify.h ++++ b/include/grub/verify.h +@@ -24,6 +24,7 @@ + + enum grub_verify_flags + { ++ GRUB_VERIFY_FLAGS_NONE = 0, + GRUB_VERIFY_FLAGS_SKIP_VERIFICATION = 1, + GRUB_VERIFY_FLAGS_SINGLE_CHUNK = 2, + /* Defer verification to another authority. */ diff --git a/grub/grub-efi/debian/patches/0021-video-readers-Add-artificial-limit-to-image-dimensio.patch b/grub/grub-efi/debian/patches/0021-video-readers-Add-artificial-limit-to-image-dimensio.patch new file mode 100644 index 000000000..99c2f5a7f --- /dev/null +++ b/grub/grub-efi/debian/patches/0021-video-readers-Add-artificial-limit-to-image-dimensio.patch @@ -0,0 +1,109 @@ +From a85714545fe57a86d14ee231a4cd312158101d43 Mon Sep 17 00:00:00 2001 +From: Alec Brown +Date: Wed, 26 Oct 2022 20:16:44 -0400 +Subject: [PATCH 01/14] video/readers: Add artificial limit to image dimensions + +In grub-core/video/readers/jpeg.c, the height and width of a JPEG image don't +have an upper limit for how big the JPEG image can be. In Coverity, this is +getting flagged as an untrusted loop bound. This issue can also seen in PNG and +TGA format images as well but Coverity isn't flagging it. To prevent this, the +constant IMAGE_HW_MAX_PX is being added to include/grub/bitmap.h, which has +a value of 16384, to act as an artificial limit and restrict the height and +width of images. This value was picked as it is double the current max +resolution size, which is 8K. + +Fixes: CID 292450 + +Signed-off-by: Alec Brown +Reviewed-by: Darren Kenny +Reviewed-by: Daniel Kiper +--- + docs/grub.texi | 3 ++- + grub-core/video/readers/jpeg.c | 6 +++++- + grub-core/video/readers/png.c | 6 +++++- + grub-core/video/readers/tga.c | 7 +++++++ + include/grub/bitmap.h | 2 ++ + 5 files changed, 21 insertions(+), 3 deletions(-) + +diff --git a/docs/grub.texi b/docs/grub.texi +index 0dbbdc374..2d6cd8358 100644 +--- a/docs/grub.texi ++++ b/docs/grub.texi +@@ -1515,7 +1515,8 @@ resolution. @xref{gfxmode}. + Set a background image for use with the @samp{gfxterm} graphical terminal. + The value of this option must be a file readable by GRUB at boot time, and + it must end with @file{.png}, @file{.tga}, @file{.jpg}, or @file{.jpeg}. +-The image will be scaled if necessary to fit the screen. ++The image will be scaled if necessary to fit the screen. Image height and ++width will be restricted by an artificial limit of 16384. + + @item GRUB_THEME + Set a theme for use with the @samp{gfxterm} graphical terminal. +diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c +index 09596fbf5..ae634fd41 100644 +--- a/grub-core/video/readers/jpeg.c ++++ b/grub-core/video/readers/jpeg.c +@@ -346,7 +346,11 @@ grub_jpeg_decode_sof (struct grub_jpeg_data *data) + data->image_height = grub_jpeg_get_word (data); + data->image_width = grub_jpeg_get_word (data); + +- if ((!data->image_height) || (!data->image_width)) ++ grub_dprintf ("jpeg", "image height: %d\n", data->image_height); ++ grub_dprintf ("jpeg", "image width: %d\n", data->image_width); ++ ++ if ((!data->image_height) || (!data->image_width) || ++ (data->image_height > IMAGE_HW_MAX_PX) || (data->image_width > IMAGE_HW_MAX_PX)) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid image size"); + + cc = grub_jpeg_get_byte (data); +diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c +index 7f2ba7849..3163e97bf 100644 +--- a/grub-core/video/readers/png.c ++++ b/grub-core/video/readers/png.c +@@ -264,7 +264,11 @@ grub_png_decode_image_header (struct grub_png_data *data) + data->image_width = grub_png_get_dword (data); + data->image_height = grub_png_get_dword (data); + +- if ((!data->image_height) || (!data->image_width)) ++ grub_dprintf ("png", "image height: %d\n", data->image_height); ++ grub_dprintf ("png", "image width: %d\n", data->image_width); ++ ++ if ((!data->image_height) || (!data->image_width) || ++ (data->image_height > IMAGE_HW_MAX_PX) || (data->image_width > IMAGE_HW_MAX_PX)) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: invalid image size"); + + color_bits = grub_png_get_byte (data); +diff --git a/grub-core/video/readers/tga.c b/grub-core/video/readers/tga.c +index a9ec3a1b6..9c35bf29d 100644 +--- a/grub-core/video/readers/tga.c ++++ b/grub-core/video/readers/tga.c +@@ -340,6 +340,13 @@ grub_video_reader_tga (struct grub_video_bitmap **bitmap, + data.image_width = grub_le_to_cpu16 (data.hdr.image_width); + data.image_height = grub_le_to_cpu16 (data.hdr.image_height); + ++ grub_dprintf ("tga", "image height: %d\n", data.image_height); ++ grub_dprintf ("tga", "image width: %d\n", data.image_width); ++ ++ /* Check image height and width are within restrictions. */ ++ if ((data.image_height > IMAGE_HW_MAX_PX) || (data.image_width > IMAGE_HW_MAX_PX)) ++ return grub_error (GRUB_ERR_BAD_FILE_TYPE, "tga: invalid image size"); ++ + /* Check that bitmap encoding is supported. */ + switch (data.hdr.image_type) + { +diff --git a/include/grub/bitmap.h b/include/grub/bitmap.h +index 5728f8ca3..149d37bfe 100644 +--- a/include/grub/bitmap.h ++++ b/include/grub/bitmap.h +@@ -24,6 +24,8 @@ + #include + #include + ++#define IMAGE_HW_MAX_PX 16384 ++ + struct grub_video_bitmap + { + /* Bitmap format description. */ +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0022-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch b/grub/grub-efi/debian/patches/0022-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch new file mode 100644 index 000000000..7330e5ab0 --- /dev/null +++ b/grub/grub-efi/debian/patches/0022-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch @@ -0,0 +1,33 @@ +From 5760fcfd466cc757540ea0d591bad6a08caeaa16 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Wed, 3 Aug 2022 19:45:33 +0800 +Subject: [PATCH 02/14] font: Reject glyphs exceeds font->max_glyph_width or + font->max_glyph_height + +Check glyph's width and height against limits specified in font's +metadata. Reject the glyph (and font) if such limits are exceeded. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 42189c325..756ca0abf 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -760,7 +760,9 @@ grub_font_get_glyph_internal (grub_font_t font, grub_uint32_t code) + || read_be_uint16 (font->file, &height) != 0 + || read_be_int16 (font->file, &xoff) != 0 + || read_be_int16 (font->file, &yoff) != 0 +- || read_be_int16 (font->file, &dwidth) != 0) ++ || read_be_int16 (font->file, &dwidth) != 0 ++ || width > font->max_char_width ++ || height > font->max_char_height) + { + remove_font (font); + return 0; +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0023-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch b/grub/grub-efi/debian/patches/0023-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch new file mode 100644 index 000000000..46ccb002b --- /dev/null +++ b/grub/grub-efi/debian/patches/0023-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch @@ -0,0 +1,110 @@ +From 941d10ad6f1dcbd12fb613002249e29ba035f985 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 00:51:20 +0800 +Subject: [PATCH 03/14] font: Fix size overflow in + grub_font_get_glyph_internal() + +The length of memory allocation and file read may overflow. This patch +fixes the problem by using safemath macros. + +There is a lot of code repetition like "(x * y + 7) / 8". It is unsafe +if overflow happens. This patch introduces grub_video_bitmap_calc_1bpp_bufsz(). +It is safe replacement for such code. It has safemath-like prototype. + +This patch also introduces grub_cast(value, pointer), it casts value to +typeof(*pointer) then store the value to *pointer. It returns true when +overflow occurs or false if there is no overflow. The semantics of arguments +and return value are designed to be consistent with other safemath macros. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 17 +++++++++++++---- + include/grub/bitmap.h | 18 ++++++++++++++++++ + include/grub/safemath.h | 2 ++ + 3 files changed, 33 insertions(+), 4 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 2f09a4a55..6a3fbebbd 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -739,7 +739,8 @@ grub_font_get_glyph_internal (grub_font_t font, grub_uint32_t code) + grub_int16_t xoff; + grub_int16_t yoff; + grub_int16_t dwidth; +- int len; ++ grub_ssize_t len; ++ grub_size_t sz; + + if (index_entry->glyph) + /* Return cached glyph. */ +@@ -768,9 +769,17 @@ grub_font_get_glyph_internal (grub_font_t font, grub_uint32_t code) + return 0; + } + +- len = (width * height + 7) / 8; +- glyph = grub_malloc (sizeof (struct grub_font_glyph) + len); +- if (!glyph) ++ /* Calculate real struct size of current glyph. */ ++ if (grub_video_bitmap_calc_1bpp_bufsz (width, height, &len) || ++ grub_add (sizeof (struct grub_font_glyph), len, &sz)) ++ { ++ remove_font (font); ++ return 0; ++ } ++ ++ /* Allocate and initialize the glyph struct. */ ++ glyph = grub_malloc (sz); ++ if (glyph == NULL) + { + remove_font (font); + return 0; +diff --git a/include/grub/bitmap.h b/include/grub/bitmap.h +index 149d37bfe..431048936 100644 +--- a/include/grub/bitmap.h ++++ b/include/grub/bitmap.h +@@ -23,6 +23,7 @@ + #include + #include + #include ++#include + + #define IMAGE_HW_MAX_PX 16384 + +@@ -81,6 +82,23 @@ grub_video_bitmap_get_height (struct grub_video_bitmap *bitmap) + return bitmap->mode_info.height; + } + ++/* ++ * Calculate and store the size of data buffer of 1bit bitmap in result. ++ * Equivalent to "*result = (width * height + 7) / 8" if no overflow occurs. ++ * Return true when overflow occurs or false if there is no overflow. ++ * This function is intentionally implemented as a macro instead of ++ * an inline function. Although a bit awkward, it preserves data types for ++ * safemath macros and reduces macro side effects as much as possible. ++ * ++ * XXX: Will report false overflow if width * height > UINT64_MAX. ++ */ ++#define grub_video_bitmap_calc_1bpp_bufsz(width, height, result) \ ++({ \ ++ grub_uint64_t _bitmap_pixels; \ ++ grub_mul ((width), (height), &_bitmap_pixels) ? 1 : \ ++ grub_cast (_bitmap_pixels / GRUB_CHAR_BIT + !!(_bitmap_pixels % GRUB_CHAR_BIT), (result)); \ ++}) ++ + void EXPORT_FUNC (grub_video_bitmap_get_mode_info) (struct grub_video_bitmap *bitmap, + struct grub_video_mode_info *mode_info); + +diff --git a/include/grub/safemath.h b/include/grub/safemath.h +index c17b89bba..bb0f826de 100644 +--- a/include/grub/safemath.h ++++ b/include/grub/safemath.h +@@ -30,6 +30,8 @@ + #define grub_sub(a, b, res) __builtin_sub_overflow(a, b, res) + #define grub_mul(a, b, res) __builtin_mul_overflow(a, b, res) + ++#define grub_cast(a, res) grub_add ((a), 0, (res)) ++ + #else + #error gcc 5.1 or newer or clang 3.8 or newer is required + #endif diff --git a/grub/grub-efi/debian/patches/0024-font-Fix-several-integer-overflows-in-grub_font_cons.patch b/grub/grub-efi/debian/patches/0024-font-Fix-several-integer-overflows-in-grub_font_cons.patch new file mode 100644 index 000000000..9d3c9b2f2 --- /dev/null +++ b/grub/grub-efi/debian/patches/0024-font-Fix-several-integer-overflows-in-grub_font_cons.patch @@ -0,0 +1,81 @@ +From b1805f251b31a9d3cfae5c3572ddfa630145dbbf Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 01:58:27 +0800 +Subject: [PATCH 04/14] font: Fix several integer overflows in + grub_font_construct_glyph() + +This patch fixes several integer overflows in grub_font_construct_glyph(). +Glyphs of invalid size, zero or leading to an overflow, are rejected. +The inconsistency between "glyph" and "max_glyph_size" when grub_malloc() +returns NULL is fixed too. + +Fixes: CVE-2022-2601 + +Reported-by: Zhang Boyang +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 29 +++++++++++++++++------------ + 1 file changed, 17 insertions(+), 12 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index e781521a7..e6548892f 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1517,6 +1517,7 @@ grub_font_construct_glyph (grub_font_t hinted_font, + struct grub_video_signed_rect bounds; + static struct grub_font_glyph *glyph = 0; + static grub_size_t max_glyph_size = 0; ++ grub_size_t cur_glyph_size; + + ensure_comb_space (glyph_id); + +@@ -1533,29 +1534,33 @@ grub_font_construct_glyph (grub_font_t hinted_font, + if (!glyph_id->ncomb && !glyph_id->attributes) + return main_glyph; + +- if (max_glyph_size < sizeof (*glyph) + (bounds.width * bounds.height + GRUB_CHAR_BIT - 1) / GRUB_CHAR_BIT) ++ if (grub_video_bitmap_calc_1bpp_bufsz (bounds.width, bounds.height, &cur_glyph_size) || ++ grub_add (sizeof (*glyph), cur_glyph_size, &cur_glyph_size)) ++ return main_glyph; ++ ++ if (max_glyph_size < cur_glyph_size) + { + grub_free (glyph); +- max_glyph_size = (sizeof (*glyph) + (bounds.width * bounds.height + GRUB_CHAR_BIT - 1) / GRUB_CHAR_BIT) * 2; +- if (max_glyph_size < 8) +- max_glyph_size = 8; +- glyph = grub_malloc (max_glyph_size); ++ if (grub_mul (cur_glyph_size, 2, &max_glyph_size)) ++ max_glyph_size = 0; ++ glyph = max_glyph_size > 0 ? grub_malloc (max_glyph_size) : NULL; + } + if (!glyph) + { ++ max_glyph_size = 0; + grub_errno = GRUB_ERR_NONE; + return main_glyph; + } + +- grub_memset (glyph, 0, sizeof (*glyph) +- + (bounds.width * bounds.height +- + GRUB_CHAR_BIT - 1) / GRUB_CHAR_BIT); ++ grub_memset (glyph, 0, cur_glyph_size); + + glyph->font = main_glyph->font; +- glyph->width = bounds.width; +- glyph->height = bounds.height; +- glyph->offset_x = bounds.x; +- glyph->offset_y = bounds.y; ++ if (bounds.width == 0 || bounds.height == 0 || ++ grub_cast (bounds.width, &glyph->width) || ++ grub_cast (bounds.height, &glyph->height) || ++ grub_cast (bounds.x, &glyph->offset_x) || ++ grub_cast (bounds.y, &glyph->offset_y)) ++ return main_glyph; + + if (glyph_id->attributes & GRUB_UNICODE_GLYPH_ATTRIBUTE_MIRROR) + grub_font_blit_glyph_mirror (glyph, main_glyph, +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0025-font-Remove-grub_font_dup_glyph.patch b/grub/grub-efi/debian/patches/0025-font-Remove-grub_font_dup_glyph.patch new file mode 100644 index 000000000..7c957a689 --- /dev/null +++ b/grub/grub-efi/debian/patches/0025-font-Remove-grub_font_dup_glyph.patch @@ -0,0 +1,42 @@ +From 25ad31c19c331aaa2dbd9bd2b2e2655de5766a9d Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 02:13:29 +0800 +Subject: [PATCH 05/14] font: Remove grub_font_dup_glyph() + +Remove grub_font_dup_glyph() since nobody is using it since 2013, and +I'm too lazy to fix the integer overflow problem in it. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 14 -------------- + 1 file changed, 14 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index e6548892f..a8576ffec 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1055,20 +1055,6 @@ grub_font_get_glyph_with_fallback (grub_font_t font, grub_uint32_t code) + return best_glyph; + } + +-#if 0 +-static struct grub_font_glyph * +-grub_font_dup_glyph (struct grub_font_glyph *glyph) +-{ +- static struct grub_font_glyph *ret; +- ret = grub_malloc (sizeof (*ret) + (glyph->width * glyph->height + 7) / 8); +- if (!ret) +- return NULL; +- grub_memcpy (ret, glyph, sizeof (*ret) +- + (glyph->width * glyph->height + 7) / 8); +- return ret; +-} +-#endif +- + /* FIXME: suboptimal. */ + static void + grub_font_blit_glyph (struct grub_font_glyph *target, +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0026-font-Fix-integer-overflow-in-ensure_comb_space.patch b/grub/grub-efi/debian/patches/0026-font-Fix-integer-overflow-in-ensure_comb_space.patch new file mode 100644 index 000000000..f35c646ad --- /dev/null +++ b/grub/grub-efi/debian/patches/0026-font-Fix-integer-overflow-in-ensure_comb_space.patch @@ -0,0 +1,48 @@ +From b2740b7e4a03bb8331d48b54b119afea76bb9d5f Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 02:27:05 +0800 +Subject: [PATCH 06/14] font: Fix integer overflow in ensure_comb_space() + +In fact it can't overflow at all because glyph_id->ncomb is only 8-bit +wide. But let's keep safe if somebody changes the width of glyph_id->ncomb +in the future. This patch also fixes the inconsistency between +render_max_comb_glyphs and render_combining_glyphs when grub_malloc() +returns NULL. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 14 +++++++++----- + 1 file changed, 9 insertions(+), 5 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index a8576ffec..9e3e0a94e 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1468,14 +1468,18 @@ ensure_comb_space (const struct grub_unicode_glyph *glyph_id) + if (glyph_id->ncomb <= render_max_comb_glyphs) + return; + +- render_max_comb_glyphs = 2 * glyph_id->ncomb; +- if (render_max_comb_glyphs < 8) ++ if (grub_mul (glyph_id->ncomb, 2, &render_max_comb_glyphs)) ++ render_max_comb_glyphs = 0; ++ if (render_max_comb_glyphs > 0 && render_max_comb_glyphs < 8) + render_max_comb_glyphs = 8; + grub_free (render_combining_glyphs); +- render_combining_glyphs = grub_malloc (render_max_comb_glyphs +- * sizeof (render_combining_glyphs[0])); ++ render_combining_glyphs = (render_max_comb_glyphs > 0) ? ++ grub_calloc (render_max_comb_glyphs, sizeof (render_combining_glyphs[0])) : NULL; + if (!render_combining_glyphs) +- grub_errno = 0; ++ { ++ render_max_comb_glyphs = 0; ++ grub_errno = GRUB_ERR_NONE; ++ } + } + + int +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0027-font-Fix-integer-overflow-in-BMP-index.patch b/grub/grub-efi/debian/patches/0027-font-Fix-integer-overflow-in-BMP-index.patch new file mode 100644 index 000000000..7d426f066 --- /dev/null +++ b/grub/grub-efi/debian/patches/0027-font-Fix-integer-overflow-in-BMP-index.patch @@ -0,0 +1,65 @@ +From afda8b60ba0712abe01ae1e64c5f7a067a0e6492 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Mon, 15 Aug 2022 02:04:58 +0800 +Subject: [PATCH 07/14] font: Fix integer overflow in BMP index + +The BMP index (font->bmp_idx) is designed as a reverse lookup table of +char entries (font->char_index), in order to speed up lookups for BMP +chars (i.e. code < 0x10000). The values in BMP index are the subscripts +of the corresponding char entries, stored in grub_uint16_t, while 0xffff +means not found. + +This patch fixes the problem of large subscript truncated to grub_uint16_t, +leading BMP index to return wrong char entry or report false miss. The +code now checks for bounds and uses BMP index as a hint, and fallbacks +to binary-search if necessary. + +On the occasion add a comment about BMP index is initialized to 0xffff. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 13 +++++++++---- + 1 file changed, 9 insertions(+), 4 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 9e3e0a94e..e4cb0d867 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -300,6 +300,8 @@ load_font_index (grub_file_t file, grub_uint32_t sect_length, struct + font->bmp_idx = grub_malloc (0x10000 * sizeof (grub_uint16_t)); + if (!font->bmp_idx) + return 1; ++ ++ /* Init the BMP index array to 0xffff. */ + grub_memset (font->bmp_idx, 0xff, 0x10000 * sizeof (grub_uint16_t)); + + +@@ -328,7 +330,7 @@ load_font_index (grub_file_t file, grub_uint32_t sect_length, struct + return 1; + } + +- if (entry->code < 0x10000) ++ if (entry->code < 0x10000 && i < 0xffff) + font->bmp_idx[entry->code] = i; + + last_code = entry->code; +@@ -696,9 +698,12 @@ find_glyph (const grub_font_t font, grub_uint32_t code) + /* Use BMP index if possible. */ + if (code < 0x10000 && font->bmp_idx) + { +- if (font->bmp_idx[code] == 0xffff) +- return 0; +- return &table[font->bmp_idx[code]]; ++ if (font->bmp_idx[code] < 0xffff) ++ return &table[font->bmp_idx[code]]; ++ /* ++ * When we are here then lookup in BMP index result in miss, ++ * fallthough to binary-search. ++ */ + } + + /* Do a binary search in `char_index', which is ordered by code point. */ +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0028-font-Fix-integer-underflow-in-binary-search-of-char-.patch b/grub/grub-efi/debian/patches/0028-font-Fix-integer-underflow-in-binary-search-of-char-.patch new file mode 100644 index 000000000..0db8d15bb --- /dev/null +++ b/grub/grub-efi/debian/patches/0028-font-Fix-integer-underflow-in-binary-search-of-char-.patch @@ -0,0 +1,86 @@ +From c140a086838e7c9af87842036f891b8393a8c4bc Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Sun, 14 Aug 2022 18:09:38 +0800 +Subject: [PATCH 08/14] font: Fix integer underflow in binary search of char + index + +If search target is less than all entries in font->index then "hi" +variable is set to -1, which translates to SIZE_MAX and leads to errors. + +This patch fixes the problem by replacing the entire binary search code +with the libstdc++'s std::lower_bound() implementation. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 40 ++++++++++++++++++++++------------------ + 1 file changed, 22 insertions(+), 18 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index e4cb0d867..abd412a5e 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -688,12 +688,12 @@ read_be_int16 (grub_file_t file, grub_int16_t * value) + static inline struct char_index_entry * + find_glyph (const grub_font_t font, grub_uint32_t code) + { +- struct char_index_entry *table; +- grub_size_t lo; +- grub_size_t hi; +- grub_size_t mid; ++ struct char_index_entry *table, *first, *end; ++ grub_size_t len; + + table = font->char_index; ++ if (table == NULL) ++ return NULL; + + /* Use BMP index if possible. */ + if (code < 0x10000 && font->bmp_idx) +@@ -706,25 +706,29 @@ find_glyph (const grub_font_t font, grub_uint32_t code) + */ + } + +- /* Do a binary search in `char_index', which is ordered by code point. */ +- lo = 0; +- hi = font->num_chars - 1; +- +- if (!table) +- return 0; ++ /* ++ * Do a binary search in char_index which is ordered by code point. ++ * The code below is the same as libstdc++'s std::lower_bound(). ++ */ ++ first = table; ++ len = font->num_chars; ++ end = first + len; + +- while (lo <= hi) ++ while (len > 0) + { +- mid = lo + (hi - lo) / 2; +- if (code < table[mid].code) +- hi = mid - 1; +- else if (code > table[mid].code) +- lo = mid + 1; ++ grub_size_t half = len >> 1; ++ struct char_index_entry *middle = first + half; ++ ++ if (middle->code < code) ++ { ++ first = middle + 1; ++ len = len - half - 1; ++ } + else +- return &table[mid]; ++ len = half; + } + +- return 0; ++ return (first < end && first->code == code) ? first : NULL; + } + + /* Get a glyph for the Unicode character CODE in FONT. The glyph is loaded +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0029-kern-efi-sb-Enforce-verification-of-font-files.patch b/grub/grub-efi/debian/patches/0029-kern-efi-sb-Enforce-verification-of-font-files.patch new file mode 100644 index 000000000..705835ac1 --- /dev/null +++ b/grub/grub-efi/debian/patches/0029-kern-efi-sb-Enforce-verification-of-font-files.patch @@ -0,0 +1,54 @@ +From 630deb8c0d8b02b670ced4b7030414bcf17aa080 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Sun, 14 Aug 2022 15:51:54 +0800 +Subject: [PATCH 09/14] kern/efi/sb: Enforce verification of font files + +As a mitigation and hardening measure enforce verification of font +files. Then only trusted font files can be load. This will reduce the +attack surface at cost of losing the ability of end-users to customize +fonts if e.g. UEFI Secure Boot is enabled. Vendors can always customize +fonts because they have ability to pack fonts into their GRUB bundles. + +This goal is achieved by: + + * Removing GRUB_FILE_TYPE_FONT from shim lock verifier's + skip-verification list. + + * Adding GRUB_FILE_TYPE_FONT to lockdown verifier's defer-auth list, + so font files must be verified by a verifier before they can be loaded. + +Suggested-by: Daniel Kiper +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/kern/efi/sb.c | 1 - + grub-core/kern/lockdown.c | 1 + + 2 files changed, 1 insertion(+), 1 deletion(-) + +diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c +index 89c4bb3fd..db42c2539 100644 +--- a/grub-core/kern/efi/sb.c ++++ b/grub-core/kern/efi/sb.c +@@ -145,7 +145,6 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), + case GRUB_FILE_TYPE_PRINT_BLOCKLIST: + case GRUB_FILE_TYPE_TESTLOAD: + case GRUB_FILE_TYPE_GET_SIZE: +- case GRUB_FILE_TYPE_FONT: + case GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY: + case GRUB_FILE_TYPE_CAT: + case GRUB_FILE_TYPE_HEXCAT: +diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c +index 0bc70fd42..af6d493cd 100644 +--- a/grub-core/kern/lockdown.c ++++ b/grub-core/kern/lockdown.c +@@ -51,6 +51,7 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: + case GRUB_FILE_TYPE_ACPI_TABLE: + case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: ++ case GRUB_FILE_TYPE_FONT: + *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; + + /* Fall through. */ +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0030-fbutil-Fix-integer-overflow.patch b/grub/grub-efi/debian/patches/0030-fbutil-Fix-integer-overflow.patch new file mode 100644 index 000000000..8dcac96c4 --- /dev/null +++ b/grub/grub-efi/debian/patches/0030-fbutil-Fix-integer-overflow.patch @@ -0,0 +1,85 @@ +From 50a11a81bc842c58962244a2dc86bbd31a426e12 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Tue, 6 Sep 2022 03:03:21 +0800 +Subject: [PATCH 10/14] fbutil: Fix integer overflow + +Expressions like u64 = u32 * u32 are unsafe because their products are +truncated to u32 even if left hand side is u64. This patch fixes all +problems like that one in fbutil. + +To get right result not only left hand side have to be u64 but it's also +necessary to cast at least one of the operands of all leaf operators of +right hand side to u64, e.g. u64 = u32 * u32 + u32 * u32 should be +u64 = (u64)u32 * u32 + (u64)u32 * u32. + +For 1-bit bitmaps grub_uint64_t have to be used. It's safe because any +combination of values in (grub_uint64_t)u32 * u32 + u32 expression will +not overflow grub_uint64_t. + +Other expressions like ptr + u32 * u32 + u32 * u32 are also vulnerable. +They should be ptr + (grub_addr_t)u32 * u32 + (grub_addr_t)u32 * u32. + +This patch also adds a comment to grub_video_fb_get_video_ptr() which +says it's arguments must be valid and no sanity check is performed +(like its siblings in grub-core/video/fb/fbutil.c). + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/video/fb/fbutil.c | 4 ++-- + include/grub/fbutil.h | 13 +++++++++---- + 2 files changed, 11 insertions(+), 6 deletions(-) + +diff --git a/grub-core/video/fb/fbutil.c b/grub-core/video/fb/fbutil.c +index b98bb51fe..25ef39f47 100644 +--- a/grub-core/video/fb/fbutil.c ++++ b/grub-core/video/fb/fbutil.c +@@ -67,7 +67,7 @@ get_pixel (struct grub_video_fbblit_info *source, + case 1: + if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED) + { +- int bit_index = y * source->mode_info->width + x; ++ grub_uint64_t bit_index = (grub_uint64_t) y * source->mode_info->width + x; + grub_uint8_t *ptr = source->data + bit_index / 8; + int bit_pos = 7 - bit_index % 8; + color = (*ptr >> bit_pos) & 0x01; +@@ -138,7 +138,7 @@ set_pixel (struct grub_video_fbblit_info *source, + case 1: + if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED) + { +- int bit_index = y * source->mode_info->width + x; ++ grub_uint64_t bit_index = (grub_uint64_t) y * source->mode_info->width + x; + grub_uint8_t *ptr = source->data + bit_index / 8; + int bit_pos = 7 - bit_index % 8; + *ptr = (*ptr & ~(1 << bit_pos)) | ((color & 0x01) << bit_pos); +diff --git a/include/grub/fbutil.h b/include/grub/fbutil.h +index 4205eb917..78a1ab3b4 100644 +--- a/include/grub/fbutil.h ++++ b/include/grub/fbutil.h +@@ -31,14 +31,19 @@ struct grub_video_fbblit_info + grub_uint8_t *data; + }; + +-/* Don't use for 1-bit bitmaps, addressing needs to be done at the bit level +- and it doesn't make sense, in general, to ask for a pointer +- to a particular pixel's data. */ ++/* ++ * Don't use for 1-bit bitmaps, addressing needs to be done at the bit level ++ * and it doesn't make sense, in general, to ask for a pointer ++ * to a particular pixel's data. ++ * ++ * This function assumes that bounds checking has been done in previous phase ++ * and they are opted out in here. ++ */ + static inline void * + grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source, + unsigned int x, unsigned int y) + { +- return source->data + y * source->mode_info->pitch + x * source->mode_info->bytes_per_pixel; ++ return source->data + (grub_addr_t) y * source->mode_info->pitch + (grub_addr_t) x * source->mode_info->bytes_per_pixel; + } + + /* Advance pointer by VAL bytes. If there is no unaligned access available, +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0031-font-Fix-an-integer-underflow-in-blit_comb.patch b/grub/grub-efi/debian/patches/0031-font-Fix-an-integer-underflow-in-blit_comb.patch new file mode 100644 index 000000000..3a2b02cba --- /dev/null +++ b/grub/grub-efi/debian/patches/0031-font-Fix-an-integer-underflow-in-blit_comb.patch @@ -0,0 +1,91 @@ +From 6d2668dea3774ed74c4cd1eadd146f1b846bc3d4 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Mon, 24 Oct 2022 08:05:35 +0800 +Subject: [PATCH 11/14] font: Fix an integer underflow in blit_comb() + +The expression (ctx.bounds.height - combining_glyphs[i]->height) / 2 may +evaluate to a very big invalid value even if both ctx.bounds.height and +combining_glyphs[i]->height are small integers. For example, if +ctx.bounds.height is 10 and combining_glyphs[i]->height is 12, this +expression evaluates to 2147483647 (expected -1). This is because +coordinates are allowed to be negative but ctx.bounds.height is an +unsigned int. So, the subtraction operates on unsigned ints and +underflows to a very big value. The division makes things even worse. +The quotient is still an invalid value even if converted back to int. + +This patch fixes the problem by casting ctx.bounds.height to int. As +a result the subtraction will operate on int and grub_uint16_t which +will be promoted to an int. So, the underflow will no longer happen. Other +uses of ctx.bounds.height (and ctx.bounds.width) are also casted to int, +to ensure coordinates are always calculated on signed integers. + +Fixes: CVE-2022-3775 + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index abd412a5e..3d3d803e8 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1203,12 +1203,12 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + ctx.bounds.height = main_glyph->height; + + above_rightx = main_glyph->offset_x + main_glyph->width; +- above_righty = ctx.bounds.y + ctx.bounds.height; ++ above_righty = ctx.bounds.y + (int) ctx.bounds.height; + + above_leftx = main_glyph->offset_x; +- above_lefty = ctx.bounds.y + ctx.bounds.height; ++ above_lefty = ctx.bounds.y + (int) ctx.bounds.height; + +- below_rightx = ctx.bounds.x + ctx.bounds.width; ++ below_rightx = ctx.bounds.x + (int) ctx.bounds.width; + below_righty = ctx.bounds.y; + + comb = grub_unicode_get_comb (glyph_id); +@@ -1221,7 +1221,7 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + + if (!combining_glyphs[i]) + continue; +- targetx = (ctx.bounds.width - combining_glyphs[i]->width) / 2 + ctx.bounds.x; ++ targetx = ((int) ctx.bounds.width - combining_glyphs[i]->width) / 2 + ctx.bounds.x; + /* CGJ is to avoid diacritics reordering. */ + if (comb[i].code + == GRUB_UNICODE_COMBINING_GRAPHEME_JOINER) +@@ -1231,8 +1231,8 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + case GRUB_UNICODE_COMB_OVERLAY: + do_blit (combining_glyphs[i], + targetx, +- (ctx.bounds.height - combining_glyphs[i]->height) / 2 +- - (ctx.bounds.height + ctx.bounds.y), &ctx); ++ ((int) ctx.bounds.height - combining_glyphs[i]->height) / 2 ++ - ((int) ctx.bounds.height + ctx.bounds.y), &ctx); + if (min_devwidth < combining_glyphs[i]->width) + min_devwidth = combining_glyphs[i]->width; + break; +@@ -1305,7 +1305,7 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + /* Fallthrough. */ + case GRUB_UNICODE_STACK_ATTACHED_ABOVE: + do_blit (combining_glyphs[i], targetx, +- -(ctx.bounds.height + ctx.bounds.y + space ++ -((int) ctx.bounds.height + ctx.bounds.y + space + + combining_glyphs[i]->height), &ctx); + if (min_devwidth < combining_glyphs[i]->width) + min_devwidth = combining_glyphs[i]->width; +@@ -1313,7 +1313,7 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + + case GRUB_UNICODE_COMB_HEBREW_DAGESH: + do_blit (combining_glyphs[i], targetx, +- -(ctx.bounds.height / 2 + ctx.bounds.y ++ -((int) ctx.bounds.height / 2 + ctx.bounds.y + + combining_glyphs[i]->height / 2), &ctx); + if (min_devwidth < combining_glyphs[i]->width) + min_devwidth = combining_glyphs[i]->width; +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0032-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch b/grub/grub-efi/debian/patches/0032-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch new file mode 100644 index 000000000..6296bf574 --- /dev/null +++ b/grub/grub-efi/debian/patches/0032-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch @@ -0,0 +1,75 @@ +From fcd7aa0c278f7cf3fb9f93f1a3966e1792339eb6 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Mon, 24 Oct 2022 07:15:41 +0800 +Subject: [PATCH 12/14] font: Harden grub_font_blit_glyph() and + grub_font_blit_glyph_mirror() + +As a mitigation and hardening measure add sanity checks to +grub_font_blit_glyph() and grub_font_blit_glyph_mirror(). This patch +makes these two functions do nothing if target blitting area isn't fully +contained in target bitmap. Therefore, if complex calculations in caller +overflows and malicious coordinates are given, we are still safe because +any coordinates which result in out-of-bound-write are rejected. However, +this patch only checks for invalid coordinates, and doesn't provide any +protection against invalid source glyph or destination glyph, e.g. +mismatch between glyph size and buffer size. + +This hardening measure is designed to mitigate possible overflows in +blit_comb(). If overflow occurs, it may return invalid bounding box +during dry run and call grub_font_blit_glyph() with malicious +coordinates during actual blitting. However, we are still safe because +the scratch glyph itself is valid, although its size makes no sense, and +any invalid coordinates are rejected. + +It would be better to call grub_fatal() if illegal parameter is detected. +However, doing this may end up in a dangerous recursion because grub_fatal() +would print messages to the screen and we are in the progress of drawing +characters on the screen. + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 14 ++++++++++++++ + 1 file changed, 14 insertions(+) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 3d3d803e8..cf15dc2f9 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1069,8 +1069,15 @@ static void + grub_font_blit_glyph (struct grub_font_glyph *target, + struct grub_font_glyph *src, unsigned dx, unsigned dy) + { ++ grub_uint16_t max_x, max_y; + unsigned src_bit, tgt_bit, src_byte, tgt_byte; + unsigned i, j; ++ ++ /* Harden against out-of-bound writes. */ ++ if ((grub_add (dx, src->width, &max_x) || max_x > target->width) || ++ (grub_add (dy, src->height, &max_y) || max_y > target->height)) ++ return; ++ + for (i = 0; i < src->height; i++) + { + src_bit = (src->width * i) % 8; +@@ -1102,9 +1109,16 @@ grub_font_blit_glyph_mirror (struct grub_font_glyph *target, + struct grub_font_glyph *src, + unsigned dx, unsigned dy) + { ++ grub_uint16_t max_x, max_y; + unsigned tgt_bit, src_byte, tgt_byte; + signed src_bit; + unsigned i, j; ++ ++ /* Harden against out-of-bound writes. */ ++ if ((grub_add (dx, src->width, &max_x) || max_x > target->width) || ++ (grub_add (dy, src->height, &max_y) || max_y > target->height)) ++ return; ++ + for (i = 0; i < src->height; i++) + { + src_bit = (src->width * i + src->width - 1) % 8; +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0033-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch b/grub/grub-efi/debian/patches/0033-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch new file mode 100644 index 000000000..2db665fcf --- /dev/null +++ b/grub/grub-efi/debian/patches/0033-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch @@ -0,0 +1,36 @@ +From dd539d695482069d28b40f2d3821f710cdcf6ee6 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 28 Oct 2022 17:29:16 +0800 +Subject: [PATCH 13/14] font: Assign null_font to glyphs in ascii_font_glyph[] + +The calculations in blit_comb() need information from glyph's font, e.g. +grub_font_get_xheight(main_glyph->font). However, main_glyph->font is +NULL if main_glyph comes from ascii_font_glyph[]. Therefore +grub_font_get_*() crashes because of NULL pointer. + +There is already a solution, the null_font. So, assign it to those glyphs +in ascii_font_glyph[]. + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index cf15dc2f9..3821937e6 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -137,7 +137,7 @@ ascii_glyph_lookup (grub_uint32_t code) + ascii_font_glyph[current]->offset_x = 0; + ascii_font_glyph[current]->offset_y = -2; + ascii_font_glyph[current]->device_width = 8; +- ascii_font_glyph[current]->font = NULL; ++ ascii_font_glyph[current]->font = &null_font; + + grub_memcpy (ascii_font_glyph[current]->bitmap, + &ascii_bitmaps[current * ASCII_BITMAP_SIZE], +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/0034-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch b/grub/grub-efi/debian/patches/0034-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch new file mode 100644 index 000000000..a94450aed --- /dev/null +++ b/grub/grub-efi/debian/patches/0034-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch @@ -0,0 +1,55 @@ +From da90d62316a3b105d2fbd7334d6521936bd6dcf6 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 28 Oct 2022 21:31:39 +0800 +Subject: [PATCH 14/14] normal/charset: Fix an integer overflow in + grub_unicode_aglomerate_comb() + +The out->ncomb is a bit-field of 8 bits. So, the max possible value is 255. +However, code in grub_unicode_aglomerate_comb() doesn't check for an +overflow when incrementing out->ncomb. If out->ncomb is already 255, +after incrementing it will get 0 instead of 256, and cause illegal +memory access in subsequent processing. + +This patch introduces GRUB_UNICODE_NCOMB_MAX to represent the max +acceptable value of ncomb. The code now checks for this limit and +ignores additional combining characters when limit is reached. + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/normal/charset.c | 3 +++ + include/grub/unicode.h | 2 ++ + 2 files changed, 5 insertions(+) + +diff --git a/grub-core/normal/charset.c b/grub-core/normal/charset.c +index 000e687bd..4f6647116 100644 +--- a/grub-core/normal/charset.c ++++ b/grub-core/normal/charset.c +@@ -472,6 +472,9 @@ grub_unicode_aglomerate_comb (const grub_uint32_t *in, grub_size_t inlen, + if (!haveout) + continue; + ++ if (out->ncomb == GRUB_UNICODE_NCOMB_MAX) ++ continue; ++ + if (comb_type == GRUB_UNICODE_COMB_MC + || comb_type == GRUB_UNICODE_COMB_ME + || comb_type == GRUB_UNICODE_COMB_MN) +diff --git a/include/grub/unicode.h b/include/grub/unicode.h +index 71a4d1a54..9360b0b97 100644 +--- a/include/grub/unicode.h ++++ b/include/grub/unicode.h +@@ -147,7 +147,9 @@ struct grub_unicode_glyph + grub_uint8_t bidi_level:6; /* minimum: 6 */ + enum grub_bidi_type bidi_type:5; /* minimum: :5 */ + ++#define GRUB_UNICODE_NCOMB_MAX ((1 << 8) - 1) + unsigned ncomb:8; ++ + /* Hint by unicode subsystem how wide this character usually is. + Real width is determined by font. Set only in UTF-8 stream. */ + int estimated_width:8; +-- +2.30.2 + diff --git a/grub/grub-efi/debian/patches/series b/grub/grub-efi/debian/patches/series index 221816f6c..2d1c991fa 100644 --- a/grub/grub-efi/debian/patches/series +++ b/grub/grub-efi/debian/patches/series @@ -17,3 +17,18 @@ 0017-grub-shim-verify-Report-that-the-loaded-object-is-ve.patch 0018-grub-verify-Add-strict_security-variable.patch 0019-Disable-inside-lockdown-and-shim_lock-verifiers.patch +0020-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch +0021-video-readers-Add-artificial-limit-to-image-dimensio.patch +0022-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch +0023-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch +0024-font-Fix-several-integer-overflows-in-grub_font_cons.patch +0025-font-Remove-grub_font_dup_glyph.patch +0026-font-Fix-integer-overflow-in-ensure_comb_space.patch +0027-font-Fix-integer-overflow-in-BMP-index.patch +0028-font-Fix-integer-underflow-in-binary-search-of-char-.patch +0029-kern-efi-sb-Enforce-verification-of-font-files.patch +0030-fbutil-Fix-integer-overflow.patch +0031-font-Fix-an-integer-underflow-in-blit_comb.patch +0032-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch +0033-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch +0034-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch diff --git a/grub/grub2/debian/patches/0003-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch b/grub/grub2/debian/patches/0003-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch new file mode 100644 index 000000000..18ecc2585 --- /dev/null +++ b/grub/grub2/debian/patches/0003-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch @@ -0,0 +1,99 @@ +From 24e6d59ac676791507ff5267bf3bef6cbaff6aef Mon Sep 17 00:00:00 2001 +From: Julian Andres Klode +Date: Thu, 2 Dec 2021 15:03:53 +0100 +Subject: kern/efi/sb: Reject non-kernel files in the shim_lock verifier + +We must not allow other verifiers to pass things like the GRUB modules. +Instead of maintaining a blocklist, maintain an allowlist of things +that we do not care about. + +This allowlist really should be made reusable, and shared by the +lockdown verifier, but this is the minimal patch addressing +security concerns where the TPM verifier was able to mark modules +as verified (or the OpenPGP verifier for that matter), when it +should not do so on shim-powered secure boot systems. + +Fixes: CVE-2022-28735 + +Signed-off-by: Julian Andres Klode +Reviewed-by: Daniel Kiper +--- + grub-core/kern/efi/sb.c | 39 ++++++++++++++++++++++++++++++++++++--- + include/grub/verify.h | 1 + + 2 files changed, 37 insertions(+), 3 deletions(-) + +diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c +index c52ec6226..89c4bb3fd 100644 +--- a/grub-core/kern/efi/sb.c ++++ b/grub-core/kern/efi/sb.c +@@ -119,10 +119,11 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), + void **context __attribute__ ((unused)), + enum grub_verify_flags *flags) + { +- *flags = GRUB_VERIFY_FLAGS_SKIP_VERIFICATION; ++ *flags = GRUB_VERIFY_FLAGS_NONE; + + switch (type & GRUB_FILE_TYPE_MASK) + { ++ /* Files we check. */ + case GRUB_FILE_TYPE_LINUX_KERNEL: + case GRUB_FILE_TYPE_MULTIBOOT_KERNEL: + case GRUB_FILE_TYPE_BSD_KERNEL: +@@ -130,11 +131,43 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), + case GRUB_FILE_TYPE_PLAN9_KERNEL: + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: + *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK; ++ return GRUB_ERR_NONE; + +- /* Fall through. */ ++ /* Files that do not affect secureboot state. */ ++ case GRUB_FILE_TYPE_NONE: ++ case GRUB_FILE_TYPE_LOOPBACK: ++ case GRUB_FILE_TYPE_LINUX_INITRD: ++ case GRUB_FILE_TYPE_OPENBSD_RAMDISK: ++ case GRUB_FILE_TYPE_XNU_RAMDISK: ++ case GRUB_FILE_TYPE_SIGNATURE: ++ case GRUB_FILE_TYPE_PUBLIC_KEY: ++ case GRUB_FILE_TYPE_PUBLIC_KEY_TRUST: ++ case GRUB_FILE_TYPE_PRINT_BLOCKLIST: ++ case GRUB_FILE_TYPE_TESTLOAD: ++ case GRUB_FILE_TYPE_GET_SIZE: ++ case GRUB_FILE_TYPE_FONT: ++ case GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY: ++ case GRUB_FILE_TYPE_CAT: ++ case GRUB_FILE_TYPE_HEXCAT: ++ case GRUB_FILE_TYPE_CMP: ++ case GRUB_FILE_TYPE_HASHLIST: ++ case GRUB_FILE_TYPE_TO_HASH: ++ case GRUB_FILE_TYPE_KEYBOARD_LAYOUT: ++ case GRUB_FILE_TYPE_PIXMAP: ++ case GRUB_FILE_TYPE_GRUB_MODULE_LIST: ++ case GRUB_FILE_TYPE_CONFIG: ++ case GRUB_FILE_TYPE_THEME: ++ case GRUB_FILE_TYPE_GETTEXT_CATALOG: ++ case GRUB_FILE_TYPE_FS_SEARCH: ++ case GRUB_FILE_TYPE_LOADENV: ++ case GRUB_FILE_TYPE_SAVEENV: ++ case GRUB_FILE_TYPE_VERIFY_SIGNATURE: ++ *flags = GRUB_VERIFY_FLAGS_SKIP_VERIFICATION; ++ return GRUB_ERR_NONE; + ++ /* Other files. */ + default: +- return GRUB_ERR_NONE; ++ return grub_error (GRUB_ERR_ACCESS_DENIED, N_("prohibited by secure boot policy")); + } + } + +diff --git a/include/grub/verify.h b/include/grub/verify.h +index 6fde244fc..67448165f 100644 +--- a/include/grub/verify.h ++++ b/include/grub/verify.h +@@ -24,6 +24,7 @@ + + enum grub_verify_flags + { ++ GRUB_VERIFY_FLAGS_NONE = 0, + GRUB_VERIFY_FLAGS_SKIP_VERIFICATION = 1, + GRUB_VERIFY_FLAGS_SINGLE_CHUNK = 2, + /* Defer verification to another authority. */ diff --git a/grub/grub2/debian/patches/0004-video-readers-Add-artificial-limit-to-image-dimensio.patch b/grub/grub2/debian/patches/0004-video-readers-Add-artificial-limit-to-image-dimensio.patch new file mode 100644 index 000000000..99c2f5a7f --- /dev/null +++ b/grub/grub2/debian/patches/0004-video-readers-Add-artificial-limit-to-image-dimensio.patch @@ -0,0 +1,109 @@ +From a85714545fe57a86d14ee231a4cd312158101d43 Mon Sep 17 00:00:00 2001 +From: Alec Brown +Date: Wed, 26 Oct 2022 20:16:44 -0400 +Subject: [PATCH 01/14] video/readers: Add artificial limit to image dimensions + +In grub-core/video/readers/jpeg.c, the height and width of a JPEG image don't +have an upper limit for how big the JPEG image can be. In Coverity, this is +getting flagged as an untrusted loop bound. This issue can also seen in PNG and +TGA format images as well but Coverity isn't flagging it. To prevent this, the +constant IMAGE_HW_MAX_PX is being added to include/grub/bitmap.h, which has +a value of 16384, to act as an artificial limit and restrict the height and +width of images. This value was picked as it is double the current max +resolution size, which is 8K. + +Fixes: CID 292450 + +Signed-off-by: Alec Brown +Reviewed-by: Darren Kenny +Reviewed-by: Daniel Kiper +--- + docs/grub.texi | 3 ++- + grub-core/video/readers/jpeg.c | 6 +++++- + grub-core/video/readers/png.c | 6 +++++- + grub-core/video/readers/tga.c | 7 +++++++ + include/grub/bitmap.h | 2 ++ + 5 files changed, 21 insertions(+), 3 deletions(-) + +diff --git a/docs/grub.texi b/docs/grub.texi +index 0dbbdc374..2d6cd8358 100644 +--- a/docs/grub.texi ++++ b/docs/grub.texi +@@ -1515,7 +1515,8 @@ resolution. @xref{gfxmode}. + Set a background image for use with the @samp{gfxterm} graphical terminal. + The value of this option must be a file readable by GRUB at boot time, and + it must end with @file{.png}, @file{.tga}, @file{.jpg}, or @file{.jpeg}. +-The image will be scaled if necessary to fit the screen. ++The image will be scaled if necessary to fit the screen. Image height and ++width will be restricted by an artificial limit of 16384. + + @item GRUB_THEME + Set a theme for use with the @samp{gfxterm} graphical terminal. +diff --git a/grub-core/video/readers/jpeg.c b/grub-core/video/readers/jpeg.c +index 09596fbf5..ae634fd41 100644 +--- a/grub-core/video/readers/jpeg.c ++++ b/grub-core/video/readers/jpeg.c +@@ -346,7 +346,11 @@ grub_jpeg_decode_sof (struct grub_jpeg_data *data) + data->image_height = grub_jpeg_get_word (data); + data->image_width = grub_jpeg_get_word (data); + +- if ((!data->image_height) || (!data->image_width)) ++ grub_dprintf ("jpeg", "image height: %d\n", data->image_height); ++ grub_dprintf ("jpeg", "image width: %d\n", data->image_width); ++ ++ if ((!data->image_height) || (!data->image_width) || ++ (data->image_height > IMAGE_HW_MAX_PX) || (data->image_width > IMAGE_HW_MAX_PX)) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, "jpeg: invalid image size"); + + cc = grub_jpeg_get_byte (data); +diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c +index 7f2ba7849..3163e97bf 100644 +--- a/grub-core/video/readers/png.c ++++ b/grub-core/video/readers/png.c +@@ -264,7 +264,11 @@ grub_png_decode_image_header (struct grub_png_data *data) + data->image_width = grub_png_get_dword (data); + data->image_height = grub_png_get_dword (data); + +- if ((!data->image_height) || (!data->image_width)) ++ grub_dprintf ("png", "image height: %d\n", data->image_height); ++ grub_dprintf ("png", "image width: %d\n", data->image_width); ++ ++ if ((!data->image_height) || (!data->image_width) || ++ (data->image_height > IMAGE_HW_MAX_PX) || (data->image_width > IMAGE_HW_MAX_PX)) + return grub_error (GRUB_ERR_BAD_FILE_TYPE, "png: invalid image size"); + + color_bits = grub_png_get_byte (data); +diff --git a/grub-core/video/readers/tga.c b/grub-core/video/readers/tga.c +index a9ec3a1b6..9c35bf29d 100644 +--- a/grub-core/video/readers/tga.c ++++ b/grub-core/video/readers/tga.c +@@ -340,6 +340,13 @@ grub_video_reader_tga (struct grub_video_bitmap **bitmap, + data.image_width = grub_le_to_cpu16 (data.hdr.image_width); + data.image_height = grub_le_to_cpu16 (data.hdr.image_height); + ++ grub_dprintf ("tga", "image height: %d\n", data.image_height); ++ grub_dprintf ("tga", "image width: %d\n", data.image_width); ++ ++ /* Check image height and width are within restrictions. */ ++ if ((data.image_height > IMAGE_HW_MAX_PX) || (data.image_width > IMAGE_HW_MAX_PX)) ++ return grub_error (GRUB_ERR_BAD_FILE_TYPE, "tga: invalid image size"); ++ + /* Check that bitmap encoding is supported. */ + switch (data.hdr.image_type) + { +diff --git a/include/grub/bitmap.h b/include/grub/bitmap.h +index 5728f8ca3..149d37bfe 100644 +--- a/include/grub/bitmap.h ++++ b/include/grub/bitmap.h +@@ -24,6 +24,8 @@ + #include + #include + ++#define IMAGE_HW_MAX_PX 16384 ++ + struct grub_video_bitmap + { + /* Bitmap format description. */ +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0005-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch b/grub/grub2/debian/patches/0005-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch new file mode 100644 index 000000000..7330e5ab0 --- /dev/null +++ b/grub/grub2/debian/patches/0005-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch @@ -0,0 +1,33 @@ +From 5760fcfd466cc757540ea0d591bad6a08caeaa16 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Wed, 3 Aug 2022 19:45:33 +0800 +Subject: [PATCH 02/14] font: Reject glyphs exceeds font->max_glyph_width or + font->max_glyph_height + +Check glyph's width and height against limits specified in font's +metadata. Reject the glyph (and font) if such limits are exceeded. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 42189c325..756ca0abf 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -760,7 +760,9 @@ grub_font_get_glyph_internal (grub_font_t font, grub_uint32_t code) + || read_be_uint16 (font->file, &height) != 0 + || read_be_int16 (font->file, &xoff) != 0 + || read_be_int16 (font->file, &yoff) != 0 +- || read_be_int16 (font->file, &dwidth) != 0) ++ || read_be_int16 (font->file, &dwidth) != 0 ++ || width > font->max_char_width ++ || height > font->max_char_height) + { + remove_font (font); + return 0; +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0006-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch b/grub/grub2/debian/patches/0006-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch new file mode 100644 index 000000000..46ccb002b --- /dev/null +++ b/grub/grub2/debian/patches/0006-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch @@ -0,0 +1,110 @@ +From 941d10ad6f1dcbd12fb613002249e29ba035f985 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 00:51:20 +0800 +Subject: [PATCH 03/14] font: Fix size overflow in + grub_font_get_glyph_internal() + +The length of memory allocation and file read may overflow. This patch +fixes the problem by using safemath macros. + +There is a lot of code repetition like "(x * y + 7) / 8". It is unsafe +if overflow happens. This patch introduces grub_video_bitmap_calc_1bpp_bufsz(). +It is safe replacement for such code. It has safemath-like prototype. + +This patch also introduces grub_cast(value, pointer), it casts value to +typeof(*pointer) then store the value to *pointer. It returns true when +overflow occurs or false if there is no overflow. The semantics of arguments +and return value are designed to be consistent with other safemath macros. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 17 +++++++++++++---- + include/grub/bitmap.h | 18 ++++++++++++++++++ + include/grub/safemath.h | 2 ++ + 3 files changed, 33 insertions(+), 4 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 2f09a4a55..6a3fbebbd 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -739,7 +739,8 @@ grub_font_get_glyph_internal (grub_font_t font, grub_uint32_t code) + grub_int16_t xoff; + grub_int16_t yoff; + grub_int16_t dwidth; +- int len; ++ grub_ssize_t len; ++ grub_size_t sz; + + if (index_entry->glyph) + /* Return cached glyph. */ +@@ -768,9 +769,17 @@ grub_font_get_glyph_internal (grub_font_t font, grub_uint32_t code) + return 0; + } + +- len = (width * height + 7) / 8; +- glyph = grub_malloc (sizeof (struct grub_font_glyph) + len); +- if (!glyph) ++ /* Calculate real struct size of current glyph. */ ++ if (grub_video_bitmap_calc_1bpp_bufsz (width, height, &len) || ++ grub_add (sizeof (struct grub_font_glyph), len, &sz)) ++ { ++ remove_font (font); ++ return 0; ++ } ++ ++ /* Allocate and initialize the glyph struct. */ ++ glyph = grub_malloc (sz); ++ if (glyph == NULL) + { + remove_font (font); + return 0; +diff --git a/include/grub/bitmap.h b/include/grub/bitmap.h +index 149d37bfe..431048936 100644 +--- a/include/grub/bitmap.h ++++ b/include/grub/bitmap.h +@@ -23,6 +23,7 @@ + #include + #include + #include ++#include + + #define IMAGE_HW_MAX_PX 16384 + +@@ -81,6 +82,23 @@ grub_video_bitmap_get_height (struct grub_video_bitmap *bitmap) + return bitmap->mode_info.height; + } + ++/* ++ * Calculate and store the size of data buffer of 1bit bitmap in result. ++ * Equivalent to "*result = (width * height + 7) / 8" if no overflow occurs. ++ * Return true when overflow occurs or false if there is no overflow. ++ * This function is intentionally implemented as a macro instead of ++ * an inline function. Although a bit awkward, it preserves data types for ++ * safemath macros and reduces macro side effects as much as possible. ++ * ++ * XXX: Will report false overflow if width * height > UINT64_MAX. ++ */ ++#define grub_video_bitmap_calc_1bpp_bufsz(width, height, result) \ ++({ \ ++ grub_uint64_t _bitmap_pixels; \ ++ grub_mul ((width), (height), &_bitmap_pixels) ? 1 : \ ++ grub_cast (_bitmap_pixels / GRUB_CHAR_BIT + !!(_bitmap_pixels % GRUB_CHAR_BIT), (result)); \ ++}) ++ + void EXPORT_FUNC (grub_video_bitmap_get_mode_info) (struct grub_video_bitmap *bitmap, + struct grub_video_mode_info *mode_info); + +diff --git a/include/grub/safemath.h b/include/grub/safemath.h +index c17b89bba..bb0f826de 100644 +--- a/include/grub/safemath.h ++++ b/include/grub/safemath.h +@@ -30,6 +30,8 @@ + #define grub_sub(a, b, res) __builtin_sub_overflow(a, b, res) + #define grub_mul(a, b, res) __builtin_mul_overflow(a, b, res) + ++#define grub_cast(a, res) grub_add ((a), 0, (res)) ++ + #else + #error gcc 5.1 or newer or clang 3.8 or newer is required + #endif diff --git a/grub/grub2/debian/patches/0007-font-Fix-several-integer-overflows-in-grub_font_cons.patch b/grub/grub2/debian/patches/0007-font-Fix-several-integer-overflows-in-grub_font_cons.patch new file mode 100644 index 000000000..9d3c9b2f2 --- /dev/null +++ b/grub/grub2/debian/patches/0007-font-Fix-several-integer-overflows-in-grub_font_cons.patch @@ -0,0 +1,81 @@ +From b1805f251b31a9d3cfae5c3572ddfa630145dbbf Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 01:58:27 +0800 +Subject: [PATCH 04/14] font: Fix several integer overflows in + grub_font_construct_glyph() + +This patch fixes several integer overflows in grub_font_construct_glyph(). +Glyphs of invalid size, zero or leading to an overflow, are rejected. +The inconsistency between "glyph" and "max_glyph_size" when grub_malloc() +returns NULL is fixed too. + +Fixes: CVE-2022-2601 + +Reported-by: Zhang Boyang +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 29 +++++++++++++++++------------ + 1 file changed, 17 insertions(+), 12 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index e781521a7..e6548892f 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1517,6 +1517,7 @@ grub_font_construct_glyph (grub_font_t hinted_font, + struct grub_video_signed_rect bounds; + static struct grub_font_glyph *glyph = 0; + static grub_size_t max_glyph_size = 0; ++ grub_size_t cur_glyph_size; + + ensure_comb_space (glyph_id); + +@@ -1533,29 +1534,33 @@ grub_font_construct_glyph (grub_font_t hinted_font, + if (!glyph_id->ncomb && !glyph_id->attributes) + return main_glyph; + +- if (max_glyph_size < sizeof (*glyph) + (bounds.width * bounds.height + GRUB_CHAR_BIT - 1) / GRUB_CHAR_BIT) ++ if (grub_video_bitmap_calc_1bpp_bufsz (bounds.width, bounds.height, &cur_glyph_size) || ++ grub_add (sizeof (*glyph), cur_glyph_size, &cur_glyph_size)) ++ return main_glyph; ++ ++ if (max_glyph_size < cur_glyph_size) + { + grub_free (glyph); +- max_glyph_size = (sizeof (*glyph) + (bounds.width * bounds.height + GRUB_CHAR_BIT - 1) / GRUB_CHAR_BIT) * 2; +- if (max_glyph_size < 8) +- max_glyph_size = 8; +- glyph = grub_malloc (max_glyph_size); ++ if (grub_mul (cur_glyph_size, 2, &max_glyph_size)) ++ max_glyph_size = 0; ++ glyph = max_glyph_size > 0 ? grub_malloc (max_glyph_size) : NULL; + } + if (!glyph) + { ++ max_glyph_size = 0; + grub_errno = GRUB_ERR_NONE; + return main_glyph; + } + +- grub_memset (glyph, 0, sizeof (*glyph) +- + (bounds.width * bounds.height +- + GRUB_CHAR_BIT - 1) / GRUB_CHAR_BIT); ++ grub_memset (glyph, 0, cur_glyph_size); + + glyph->font = main_glyph->font; +- glyph->width = bounds.width; +- glyph->height = bounds.height; +- glyph->offset_x = bounds.x; +- glyph->offset_y = bounds.y; ++ if (bounds.width == 0 || bounds.height == 0 || ++ grub_cast (bounds.width, &glyph->width) || ++ grub_cast (bounds.height, &glyph->height) || ++ grub_cast (bounds.x, &glyph->offset_x) || ++ grub_cast (bounds.y, &glyph->offset_y)) ++ return main_glyph; + + if (glyph_id->attributes & GRUB_UNICODE_GLYPH_ATTRIBUTE_MIRROR) + grub_font_blit_glyph_mirror (glyph, main_glyph, +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0008-font-Remove-grub_font_dup_glyph.patch b/grub/grub2/debian/patches/0008-font-Remove-grub_font_dup_glyph.patch new file mode 100644 index 000000000..7c957a689 --- /dev/null +++ b/grub/grub2/debian/patches/0008-font-Remove-grub_font_dup_glyph.patch @@ -0,0 +1,42 @@ +From 25ad31c19c331aaa2dbd9bd2b2e2655de5766a9d Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 02:13:29 +0800 +Subject: [PATCH 05/14] font: Remove grub_font_dup_glyph() + +Remove grub_font_dup_glyph() since nobody is using it since 2013, and +I'm too lazy to fix the integer overflow problem in it. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 14 -------------- + 1 file changed, 14 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index e6548892f..a8576ffec 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1055,20 +1055,6 @@ grub_font_get_glyph_with_fallback (grub_font_t font, grub_uint32_t code) + return best_glyph; + } + +-#if 0 +-static struct grub_font_glyph * +-grub_font_dup_glyph (struct grub_font_glyph *glyph) +-{ +- static struct grub_font_glyph *ret; +- ret = grub_malloc (sizeof (*ret) + (glyph->width * glyph->height + 7) / 8); +- if (!ret) +- return NULL; +- grub_memcpy (ret, glyph, sizeof (*ret) +- + (glyph->width * glyph->height + 7) / 8); +- return ret; +-} +-#endif +- + /* FIXME: suboptimal. */ + static void + grub_font_blit_glyph (struct grub_font_glyph *target, +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0009-font-Fix-integer-overflow-in-ensure_comb_space.patch b/grub/grub2/debian/patches/0009-font-Fix-integer-overflow-in-ensure_comb_space.patch new file mode 100644 index 000000000..f35c646ad --- /dev/null +++ b/grub/grub2/debian/patches/0009-font-Fix-integer-overflow-in-ensure_comb_space.patch @@ -0,0 +1,48 @@ +From b2740b7e4a03bb8331d48b54b119afea76bb9d5f Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 5 Aug 2022 02:27:05 +0800 +Subject: [PATCH 06/14] font: Fix integer overflow in ensure_comb_space() + +In fact it can't overflow at all because glyph_id->ncomb is only 8-bit +wide. But let's keep safe if somebody changes the width of glyph_id->ncomb +in the future. This patch also fixes the inconsistency between +render_max_comb_glyphs and render_combining_glyphs when grub_malloc() +returns NULL. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 14 +++++++++----- + 1 file changed, 9 insertions(+), 5 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index a8576ffec..9e3e0a94e 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1468,14 +1468,18 @@ ensure_comb_space (const struct grub_unicode_glyph *glyph_id) + if (glyph_id->ncomb <= render_max_comb_glyphs) + return; + +- render_max_comb_glyphs = 2 * glyph_id->ncomb; +- if (render_max_comb_glyphs < 8) ++ if (grub_mul (glyph_id->ncomb, 2, &render_max_comb_glyphs)) ++ render_max_comb_glyphs = 0; ++ if (render_max_comb_glyphs > 0 && render_max_comb_glyphs < 8) + render_max_comb_glyphs = 8; + grub_free (render_combining_glyphs); +- render_combining_glyphs = grub_malloc (render_max_comb_glyphs +- * sizeof (render_combining_glyphs[0])); ++ render_combining_glyphs = (render_max_comb_glyphs > 0) ? ++ grub_calloc (render_max_comb_glyphs, sizeof (render_combining_glyphs[0])) : NULL; + if (!render_combining_glyphs) +- grub_errno = 0; ++ { ++ render_max_comb_glyphs = 0; ++ grub_errno = GRUB_ERR_NONE; ++ } + } + + int +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0010-font-Fix-integer-overflow-in-BMP-index.patch b/grub/grub2/debian/patches/0010-font-Fix-integer-overflow-in-BMP-index.patch new file mode 100644 index 000000000..7d426f066 --- /dev/null +++ b/grub/grub2/debian/patches/0010-font-Fix-integer-overflow-in-BMP-index.patch @@ -0,0 +1,65 @@ +From afda8b60ba0712abe01ae1e64c5f7a067a0e6492 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Mon, 15 Aug 2022 02:04:58 +0800 +Subject: [PATCH 07/14] font: Fix integer overflow in BMP index + +The BMP index (font->bmp_idx) is designed as a reverse lookup table of +char entries (font->char_index), in order to speed up lookups for BMP +chars (i.e. code < 0x10000). The values in BMP index are the subscripts +of the corresponding char entries, stored in grub_uint16_t, while 0xffff +means not found. + +This patch fixes the problem of large subscript truncated to grub_uint16_t, +leading BMP index to return wrong char entry or report false miss. The +code now checks for bounds and uses BMP index as a hint, and fallbacks +to binary-search if necessary. + +On the occasion add a comment about BMP index is initialized to 0xffff. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 13 +++++++++---- + 1 file changed, 9 insertions(+), 4 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 9e3e0a94e..e4cb0d867 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -300,6 +300,8 @@ load_font_index (grub_file_t file, grub_uint32_t sect_length, struct + font->bmp_idx = grub_malloc (0x10000 * sizeof (grub_uint16_t)); + if (!font->bmp_idx) + return 1; ++ ++ /* Init the BMP index array to 0xffff. */ + grub_memset (font->bmp_idx, 0xff, 0x10000 * sizeof (grub_uint16_t)); + + +@@ -328,7 +330,7 @@ load_font_index (grub_file_t file, grub_uint32_t sect_length, struct + return 1; + } + +- if (entry->code < 0x10000) ++ if (entry->code < 0x10000 && i < 0xffff) + font->bmp_idx[entry->code] = i; + + last_code = entry->code; +@@ -696,9 +698,12 @@ find_glyph (const grub_font_t font, grub_uint32_t code) + /* Use BMP index if possible. */ + if (code < 0x10000 && font->bmp_idx) + { +- if (font->bmp_idx[code] == 0xffff) +- return 0; +- return &table[font->bmp_idx[code]]; ++ if (font->bmp_idx[code] < 0xffff) ++ return &table[font->bmp_idx[code]]; ++ /* ++ * When we are here then lookup in BMP index result in miss, ++ * fallthough to binary-search. ++ */ + } + + /* Do a binary search in `char_index', which is ordered by code point. */ +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0011-font-Fix-integer-underflow-in-binary-search-of-char-.patch b/grub/grub2/debian/patches/0011-font-Fix-integer-underflow-in-binary-search-of-char-.patch new file mode 100644 index 000000000..0db8d15bb --- /dev/null +++ b/grub/grub2/debian/patches/0011-font-Fix-integer-underflow-in-binary-search-of-char-.patch @@ -0,0 +1,86 @@ +From c140a086838e7c9af87842036f891b8393a8c4bc Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Sun, 14 Aug 2022 18:09:38 +0800 +Subject: [PATCH 08/14] font: Fix integer underflow in binary search of char + index + +If search target is less than all entries in font->index then "hi" +variable is set to -1, which translates to SIZE_MAX and leads to errors. + +This patch fixes the problem by replacing the entire binary search code +with the libstdc++'s std::lower_bound() implementation. + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 40 ++++++++++++++++++++++------------------ + 1 file changed, 22 insertions(+), 18 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index e4cb0d867..abd412a5e 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -688,12 +688,12 @@ read_be_int16 (grub_file_t file, grub_int16_t * value) + static inline struct char_index_entry * + find_glyph (const grub_font_t font, grub_uint32_t code) + { +- struct char_index_entry *table; +- grub_size_t lo; +- grub_size_t hi; +- grub_size_t mid; ++ struct char_index_entry *table, *first, *end; ++ grub_size_t len; + + table = font->char_index; ++ if (table == NULL) ++ return NULL; + + /* Use BMP index if possible. */ + if (code < 0x10000 && font->bmp_idx) +@@ -706,25 +706,29 @@ find_glyph (const grub_font_t font, grub_uint32_t code) + */ + } + +- /* Do a binary search in `char_index', which is ordered by code point. */ +- lo = 0; +- hi = font->num_chars - 1; +- +- if (!table) +- return 0; ++ /* ++ * Do a binary search in char_index which is ordered by code point. ++ * The code below is the same as libstdc++'s std::lower_bound(). ++ */ ++ first = table; ++ len = font->num_chars; ++ end = first + len; + +- while (lo <= hi) ++ while (len > 0) + { +- mid = lo + (hi - lo) / 2; +- if (code < table[mid].code) +- hi = mid - 1; +- else if (code > table[mid].code) +- lo = mid + 1; ++ grub_size_t half = len >> 1; ++ struct char_index_entry *middle = first + half; ++ ++ if (middle->code < code) ++ { ++ first = middle + 1; ++ len = len - half - 1; ++ } + else +- return &table[mid]; ++ len = half; + } + +- return 0; ++ return (first < end && first->code == code) ? first : NULL; + } + + /* Get a glyph for the Unicode character CODE in FONT. The glyph is loaded +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0012-kern-efi-sb-Enforce-verification-of-font-files.patch b/grub/grub2/debian/patches/0012-kern-efi-sb-Enforce-verification-of-font-files.patch new file mode 100644 index 000000000..705835ac1 --- /dev/null +++ b/grub/grub2/debian/patches/0012-kern-efi-sb-Enforce-verification-of-font-files.patch @@ -0,0 +1,54 @@ +From 630deb8c0d8b02b670ced4b7030414bcf17aa080 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Sun, 14 Aug 2022 15:51:54 +0800 +Subject: [PATCH 09/14] kern/efi/sb: Enforce verification of font files + +As a mitigation and hardening measure enforce verification of font +files. Then only trusted font files can be load. This will reduce the +attack surface at cost of losing the ability of end-users to customize +fonts if e.g. UEFI Secure Boot is enabled. Vendors can always customize +fonts because they have ability to pack fonts into their GRUB bundles. + +This goal is achieved by: + + * Removing GRUB_FILE_TYPE_FONT from shim lock verifier's + skip-verification list. + + * Adding GRUB_FILE_TYPE_FONT to lockdown verifier's defer-auth list, + so font files must be verified by a verifier before they can be loaded. + +Suggested-by: Daniel Kiper +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/kern/efi/sb.c | 1 - + grub-core/kern/lockdown.c | 1 + + 2 files changed, 1 insertion(+), 1 deletion(-) + +diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c +index 89c4bb3fd..db42c2539 100644 +--- a/grub-core/kern/efi/sb.c ++++ b/grub-core/kern/efi/sb.c +@@ -145,7 +145,6 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), + case GRUB_FILE_TYPE_PRINT_BLOCKLIST: + case GRUB_FILE_TYPE_TESTLOAD: + case GRUB_FILE_TYPE_GET_SIZE: +- case GRUB_FILE_TYPE_FONT: + case GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY: + case GRUB_FILE_TYPE_CAT: + case GRUB_FILE_TYPE_HEXCAT: +diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c +index 0bc70fd42..af6d493cd 100644 +--- a/grub-core/kern/lockdown.c ++++ b/grub-core/kern/lockdown.c +@@ -51,6 +51,7 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: + case GRUB_FILE_TYPE_ACPI_TABLE: + case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: ++ case GRUB_FILE_TYPE_FONT: + *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; + + /* Fall through. */ +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0013-fbutil-Fix-integer-overflow.patch b/grub/grub2/debian/patches/0013-fbutil-Fix-integer-overflow.patch new file mode 100644 index 000000000..8dcac96c4 --- /dev/null +++ b/grub/grub2/debian/patches/0013-fbutil-Fix-integer-overflow.patch @@ -0,0 +1,85 @@ +From 50a11a81bc842c58962244a2dc86bbd31a426e12 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Tue, 6 Sep 2022 03:03:21 +0800 +Subject: [PATCH 10/14] fbutil: Fix integer overflow + +Expressions like u64 = u32 * u32 are unsafe because their products are +truncated to u32 even if left hand side is u64. This patch fixes all +problems like that one in fbutil. + +To get right result not only left hand side have to be u64 but it's also +necessary to cast at least one of the operands of all leaf operators of +right hand side to u64, e.g. u64 = u32 * u32 + u32 * u32 should be +u64 = (u64)u32 * u32 + (u64)u32 * u32. + +For 1-bit bitmaps grub_uint64_t have to be used. It's safe because any +combination of values in (grub_uint64_t)u32 * u32 + u32 expression will +not overflow grub_uint64_t. + +Other expressions like ptr + u32 * u32 + u32 * u32 are also vulnerable. +They should be ptr + (grub_addr_t)u32 * u32 + (grub_addr_t)u32 * u32. + +This patch also adds a comment to grub_video_fb_get_video_ptr() which +says it's arguments must be valid and no sanity check is performed +(like its siblings in grub-core/video/fb/fbutil.c). + +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/video/fb/fbutil.c | 4 ++-- + include/grub/fbutil.h | 13 +++++++++---- + 2 files changed, 11 insertions(+), 6 deletions(-) + +diff --git a/grub-core/video/fb/fbutil.c b/grub-core/video/fb/fbutil.c +index b98bb51fe..25ef39f47 100644 +--- a/grub-core/video/fb/fbutil.c ++++ b/grub-core/video/fb/fbutil.c +@@ -67,7 +67,7 @@ get_pixel (struct grub_video_fbblit_info *source, + case 1: + if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED) + { +- int bit_index = y * source->mode_info->width + x; ++ grub_uint64_t bit_index = (grub_uint64_t) y * source->mode_info->width + x; + grub_uint8_t *ptr = source->data + bit_index / 8; + int bit_pos = 7 - bit_index % 8; + color = (*ptr >> bit_pos) & 0x01; +@@ -138,7 +138,7 @@ set_pixel (struct grub_video_fbblit_info *source, + case 1: + if (source->mode_info->blit_format == GRUB_VIDEO_BLIT_FORMAT_1BIT_PACKED) + { +- int bit_index = y * source->mode_info->width + x; ++ grub_uint64_t bit_index = (grub_uint64_t) y * source->mode_info->width + x; + grub_uint8_t *ptr = source->data + bit_index / 8; + int bit_pos = 7 - bit_index % 8; + *ptr = (*ptr & ~(1 << bit_pos)) | ((color & 0x01) << bit_pos); +diff --git a/include/grub/fbutil.h b/include/grub/fbutil.h +index 4205eb917..78a1ab3b4 100644 +--- a/include/grub/fbutil.h ++++ b/include/grub/fbutil.h +@@ -31,14 +31,19 @@ struct grub_video_fbblit_info + grub_uint8_t *data; + }; + +-/* Don't use for 1-bit bitmaps, addressing needs to be done at the bit level +- and it doesn't make sense, in general, to ask for a pointer +- to a particular pixel's data. */ ++/* ++ * Don't use for 1-bit bitmaps, addressing needs to be done at the bit level ++ * and it doesn't make sense, in general, to ask for a pointer ++ * to a particular pixel's data. ++ * ++ * This function assumes that bounds checking has been done in previous phase ++ * and they are opted out in here. ++ */ + static inline void * + grub_video_fb_get_video_ptr (struct grub_video_fbblit_info *source, + unsigned int x, unsigned int y) + { +- return source->data + y * source->mode_info->pitch + x * source->mode_info->bytes_per_pixel; ++ return source->data + (grub_addr_t) y * source->mode_info->pitch + (grub_addr_t) x * source->mode_info->bytes_per_pixel; + } + + /* Advance pointer by VAL bytes. If there is no unaligned access available, +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0014-font-Fix-an-integer-underflow-in-blit_comb.patch b/grub/grub2/debian/patches/0014-font-Fix-an-integer-underflow-in-blit_comb.patch new file mode 100644 index 000000000..3a2b02cba --- /dev/null +++ b/grub/grub2/debian/patches/0014-font-Fix-an-integer-underflow-in-blit_comb.patch @@ -0,0 +1,91 @@ +From 6d2668dea3774ed74c4cd1eadd146f1b846bc3d4 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Mon, 24 Oct 2022 08:05:35 +0800 +Subject: [PATCH 11/14] font: Fix an integer underflow in blit_comb() + +The expression (ctx.bounds.height - combining_glyphs[i]->height) / 2 may +evaluate to a very big invalid value even if both ctx.bounds.height and +combining_glyphs[i]->height are small integers. For example, if +ctx.bounds.height is 10 and combining_glyphs[i]->height is 12, this +expression evaluates to 2147483647 (expected -1). This is because +coordinates are allowed to be negative but ctx.bounds.height is an +unsigned int. So, the subtraction operates on unsigned ints and +underflows to a very big value. The division makes things even worse. +The quotient is still an invalid value even if converted back to int. + +This patch fixes the problem by casting ctx.bounds.height to int. As +a result the subtraction will operate on int and grub_uint16_t which +will be promoted to an int. So, the underflow will no longer happen. Other +uses of ctx.bounds.height (and ctx.bounds.width) are also casted to int, +to ensure coordinates are always calculated on signed integers. + +Fixes: CVE-2022-3775 + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 16 ++++++++-------- + 1 file changed, 8 insertions(+), 8 deletions(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index abd412a5e..3d3d803e8 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1203,12 +1203,12 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + ctx.bounds.height = main_glyph->height; + + above_rightx = main_glyph->offset_x + main_glyph->width; +- above_righty = ctx.bounds.y + ctx.bounds.height; ++ above_righty = ctx.bounds.y + (int) ctx.bounds.height; + + above_leftx = main_glyph->offset_x; +- above_lefty = ctx.bounds.y + ctx.bounds.height; ++ above_lefty = ctx.bounds.y + (int) ctx.bounds.height; + +- below_rightx = ctx.bounds.x + ctx.bounds.width; ++ below_rightx = ctx.bounds.x + (int) ctx.bounds.width; + below_righty = ctx.bounds.y; + + comb = grub_unicode_get_comb (glyph_id); +@@ -1221,7 +1221,7 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + + if (!combining_glyphs[i]) + continue; +- targetx = (ctx.bounds.width - combining_glyphs[i]->width) / 2 + ctx.bounds.x; ++ targetx = ((int) ctx.bounds.width - combining_glyphs[i]->width) / 2 + ctx.bounds.x; + /* CGJ is to avoid diacritics reordering. */ + if (comb[i].code + == GRUB_UNICODE_COMBINING_GRAPHEME_JOINER) +@@ -1231,8 +1231,8 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + case GRUB_UNICODE_COMB_OVERLAY: + do_blit (combining_glyphs[i], + targetx, +- (ctx.bounds.height - combining_glyphs[i]->height) / 2 +- - (ctx.bounds.height + ctx.bounds.y), &ctx); ++ ((int) ctx.bounds.height - combining_glyphs[i]->height) / 2 ++ - ((int) ctx.bounds.height + ctx.bounds.y), &ctx); + if (min_devwidth < combining_glyphs[i]->width) + min_devwidth = combining_glyphs[i]->width; + break; +@@ -1305,7 +1305,7 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + /* Fallthrough. */ + case GRUB_UNICODE_STACK_ATTACHED_ABOVE: + do_blit (combining_glyphs[i], targetx, +- -(ctx.bounds.height + ctx.bounds.y + space ++ -((int) ctx.bounds.height + ctx.bounds.y + space + + combining_glyphs[i]->height), &ctx); + if (min_devwidth < combining_glyphs[i]->width) + min_devwidth = combining_glyphs[i]->width; +@@ -1313,7 +1313,7 @@ blit_comb (const struct grub_unicode_glyph *glyph_id, + + case GRUB_UNICODE_COMB_HEBREW_DAGESH: + do_blit (combining_glyphs[i], targetx, +- -(ctx.bounds.height / 2 + ctx.bounds.y ++ -((int) ctx.bounds.height / 2 + ctx.bounds.y + + combining_glyphs[i]->height / 2), &ctx); + if (min_devwidth < combining_glyphs[i]->width) + min_devwidth = combining_glyphs[i]->width; +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0015-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch b/grub/grub2/debian/patches/0015-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch new file mode 100644 index 000000000..6296bf574 --- /dev/null +++ b/grub/grub2/debian/patches/0015-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch @@ -0,0 +1,75 @@ +From fcd7aa0c278f7cf3fb9f93f1a3966e1792339eb6 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Mon, 24 Oct 2022 07:15:41 +0800 +Subject: [PATCH 12/14] font: Harden grub_font_blit_glyph() and + grub_font_blit_glyph_mirror() + +As a mitigation and hardening measure add sanity checks to +grub_font_blit_glyph() and grub_font_blit_glyph_mirror(). This patch +makes these two functions do nothing if target blitting area isn't fully +contained in target bitmap. Therefore, if complex calculations in caller +overflows and malicious coordinates are given, we are still safe because +any coordinates which result in out-of-bound-write are rejected. However, +this patch only checks for invalid coordinates, and doesn't provide any +protection against invalid source glyph or destination glyph, e.g. +mismatch between glyph size and buffer size. + +This hardening measure is designed to mitigate possible overflows in +blit_comb(). If overflow occurs, it may return invalid bounding box +during dry run and call grub_font_blit_glyph() with malicious +coordinates during actual blitting. However, we are still safe because +the scratch glyph itself is valid, although its size makes no sense, and +any invalid coordinates are rejected. + +It would be better to call grub_fatal() if illegal parameter is detected. +However, doing this may end up in a dangerous recursion because grub_fatal() +would print messages to the screen and we are in the progress of drawing +characters on the screen. + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 14 ++++++++++++++ + 1 file changed, 14 insertions(+) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index 3d3d803e8..cf15dc2f9 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -1069,8 +1069,15 @@ static void + grub_font_blit_glyph (struct grub_font_glyph *target, + struct grub_font_glyph *src, unsigned dx, unsigned dy) + { ++ grub_uint16_t max_x, max_y; + unsigned src_bit, tgt_bit, src_byte, tgt_byte; + unsigned i, j; ++ ++ /* Harden against out-of-bound writes. */ ++ if ((grub_add (dx, src->width, &max_x) || max_x > target->width) || ++ (grub_add (dy, src->height, &max_y) || max_y > target->height)) ++ return; ++ + for (i = 0; i < src->height; i++) + { + src_bit = (src->width * i) % 8; +@@ -1102,9 +1109,16 @@ grub_font_blit_glyph_mirror (struct grub_font_glyph *target, + struct grub_font_glyph *src, + unsigned dx, unsigned dy) + { ++ grub_uint16_t max_x, max_y; + unsigned tgt_bit, src_byte, tgt_byte; + signed src_bit; + unsigned i, j; ++ ++ /* Harden against out-of-bound writes. */ ++ if ((grub_add (dx, src->width, &max_x) || max_x > target->width) || ++ (grub_add (dy, src->height, &max_y) || max_y > target->height)) ++ return; ++ + for (i = 0; i < src->height; i++) + { + src_bit = (src->width * i + src->width - 1) % 8; +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0016-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch b/grub/grub2/debian/patches/0016-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch new file mode 100644 index 000000000..2db665fcf --- /dev/null +++ b/grub/grub2/debian/patches/0016-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch @@ -0,0 +1,36 @@ +From dd539d695482069d28b40f2d3821f710cdcf6ee6 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 28 Oct 2022 17:29:16 +0800 +Subject: [PATCH 13/14] font: Assign null_font to glyphs in ascii_font_glyph[] + +The calculations in blit_comb() need information from glyph's font, e.g. +grub_font_get_xheight(main_glyph->font). However, main_glyph->font is +NULL if main_glyph comes from ascii_font_glyph[]. Therefore +grub_font_get_*() crashes because of NULL pointer. + +There is already a solution, the null_font. So, assign it to those glyphs +in ascii_font_glyph[]. + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/font/font.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/grub-core/font/font.c b/grub-core/font/font.c +index cf15dc2f9..3821937e6 100644 +--- a/grub-core/font/font.c ++++ b/grub-core/font/font.c +@@ -137,7 +137,7 @@ ascii_glyph_lookup (grub_uint32_t code) + ascii_font_glyph[current]->offset_x = 0; + ascii_font_glyph[current]->offset_y = -2; + ascii_font_glyph[current]->device_width = 8; +- ascii_font_glyph[current]->font = NULL; ++ ascii_font_glyph[current]->font = &null_font; + + grub_memcpy (ascii_font_glyph[current]->bitmap, + &ascii_bitmaps[current * ASCII_BITMAP_SIZE], +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/0017-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch b/grub/grub2/debian/patches/0017-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch new file mode 100644 index 000000000..a94450aed --- /dev/null +++ b/grub/grub2/debian/patches/0017-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch @@ -0,0 +1,55 @@ +From da90d62316a3b105d2fbd7334d6521936bd6dcf6 Mon Sep 17 00:00:00 2001 +From: Zhang Boyang +Date: Fri, 28 Oct 2022 21:31:39 +0800 +Subject: [PATCH 14/14] normal/charset: Fix an integer overflow in + grub_unicode_aglomerate_comb() + +The out->ncomb is a bit-field of 8 bits. So, the max possible value is 255. +However, code in grub_unicode_aglomerate_comb() doesn't check for an +overflow when incrementing out->ncomb. If out->ncomb is already 255, +after incrementing it will get 0 instead of 256, and cause illegal +memory access in subsequent processing. + +This patch introduces GRUB_UNICODE_NCOMB_MAX to represent the max +acceptable value of ncomb. The code now checks for this limit and +ignores additional combining characters when limit is reached. + +Reported-by: Daniel Axtens +Signed-off-by: Zhang Boyang +Reviewed-by: Daniel Kiper +--- + grub-core/normal/charset.c | 3 +++ + include/grub/unicode.h | 2 ++ + 2 files changed, 5 insertions(+) + +diff --git a/grub-core/normal/charset.c b/grub-core/normal/charset.c +index 000e687bd..4f6647116 100644 +--- a/grub-core/normal/charset.c ++++ b/grub-core/normal/charset.c +@@ -472,6 +472,9 @@ grub_unicode_aglomerate_comb (const grub_uint32_t *in, grub_size_t inlen, + if (!haveout) + continue; + ++ if (out->ncomb == GRUB_UNICODE_NCOMB_MAX) ++ continue; ++ + if (comb_type == GRUB_UNICODE_COMB_MC + || comb_type == GRUB_UNICODE_COMB_ME + || comb_type == GRUB_UNICODE_COMB_MN) +diff --git a/include/grub/unicode.h b/include/grub/unicode.h +index 71a4d1a54..9360b0b97 100644 +--- a/include/grub/unicode.h ++++ b/include/grub/unicode.h +@@ -147,7 +147,9 @@ struct grub_unicode_glyph + grub_uint8_t bidi_level:6; /* minimum: 6 */ + enum grub_bidi_type bidi_type:5; /* minimum: :5 */ + ++#define GRUB_UNICODE_NCOMB_MAX ((1 << 8) - 1) + unsigned ncomb:8; ++ + /* Hint by unicode subsystem how wide this character usually is. + Real width is determined by font. Set only in UTF-8 stream. */ + int estimated_width:8; +-- +2.30.2 + diff --git a/grub/grub2/debian/patches/series b/grub/grub2/debian/patches/series index 2c01fc76d..465b0f008 100644 --- a/grub/grub2/debian/patches/series +++ b/grub/grub2/debian/patches/series @@ -1,2 +1,17 @@ 0001-grub2-add-tboot.patch 0002-grub2-checking-if-loop-devices-are-available.patch +0003-kern-efi-sb-Reject-non-kernel-files-in-the-shim_lock.patch +0004-video-readers-Add-artificial-limit-to-image-dimensio.patch +0005-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch +0006-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch +0007-font-Fix-several-integer-overflows-in-grub_font_cons.patch +0008-font-Remove-grub_font_dup_glyph.patch +0009-font-Fix-integer-overflow-in-ensure_comb_space.patch +0010-font-Fix-integer-overflow-in-BMP-index.patch +0011-font-Fix-integer-underflow-in-binary-search-of-char-.patch +0012-kern-efi-sb-Enforce-verification-of-font-files.patch +0013-fbutil-Fix-integer-overflow.patch +0014-font-Fix-an-integer-underflow-in-blit_comb.patch +0015-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch +0016-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch +0017-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch