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 <li.zhou@windriver.com>
Change-Id: Ia6c58a2021a786ef92f760b3cfe035fbccedacf7
This commit is contained in:
Li Zhou 2023-05-30 16:49:58 +08:00
parent 260e448c99
commit d10d6fb187
32 changed files with 2168 additions and 0 deletions

View File

@ -0,0 +1,99 @@
From 24e6d59ac676791507ff5267bf3bef6cbaff6aef Mon Sep 17 00:00:00 2001
From: Julian Andres Klode <julian.klode@canonical.com>
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 <julian.klode@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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. */

View File

@ -0,0 +1,109 @@
From a85714545fe57a86d14ee231a4cd312158101d43 Mon Sep 17 00:00:00 2001
From: Alec Brown <alec.r.brown@oracle.com>
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 <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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 <grub/types.h>
#include <grub/video.h>
+#define IMAGE_HW_MAX_PX 16384
+
struct grub_video_bitmap
{
/* Bitmap format description. */
--
2.30.2

View File

@ -0,0 +1,33 @@
From 5760fcfd466cc757540ea0d591bad6a08caeaa16 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,110 @@
From 941d10ad6f1dcbd12fb613002249e29ba035f985 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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 <grub/symbol.h>
#include <grub/types.h>
#include <grub/video.h>
+#include <grub/safemath.h>
#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

View File

@ -0,0 +1,81 @@
From b1805f251b31a9d3cfae5c3572ddfa630145dbbf Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,42 @@
From 25ad31c19c331aaa2dbd9bd2b2e2655de5766a9d Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,48 @@
From b2740b7e4a03bb8331d48b54b119afea76bb9d5f Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,65 @@
From afda8b60ba0712abe01ae1e64c5f7a067a0e6492 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,86 @@
From c140a086838e7c9af87842036f891b8393a8c4bc Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,54 @@
From 630deb8c0d8b02b670ced4b7030414bcf17aa080 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <daniel.kiper@oracle.com>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,85 @@
From 50a11a81bc842c58962244a2dc86bbd31a426e12 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,91 @@
From 6d2668dea3774ed74c4cd1eadd146f1b846bc3d4 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,75 @@
From fcd7aa0c278f7cf3fb9f93f1a3966e1792339eb6 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,36 @@
From dd539d695482069d28b40f2d3821f710cdcf6ee6 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,55 @@
From da90d62316a3b105d2fbd7334d6521936bd6dcf6 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -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

View File

@ -0,0 +1,99 @@
From 24e6d59ac676791507ff5267bf3bef6cbaff6aef Mon Sep 17 00:00:00 2001
From: Julian Andres Klode <julian.klode@canonical.com>
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 <julian.klode@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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. */

View File

@ -0,0 +1,109 @@
From a85714545fe57a86d14ee231a4cd312158101d43 Mon Sep 17 00:00:00 2001
From: Alec Brown <alec.r.brown@oracle.com>
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 <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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 <grub/types.h>
#include <grub/video.h>
+#define IMAGE_HW_MAX_PX 16384
+
struct grub_video_bitmap
{
/* Bitmap format description. */
--
2.30.2

View File

@ -0,0 +1,33 @@
From 5760fcfd466cc757540ea0d591bad6a08caeaa16 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,110 @@
From 941d10ad6f1dcbd12fb613002249e29ba035f985 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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 <grub/symbol.h>
#include <grub/types.h>
#include <grub/video.h>
+#include <grub/safemath.h>
#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

View File

@ -0,0 +1,81 @@
From b1805f251b31a9d3cfae5c3572ddfa630145dbbf Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,42 @@
From 25ad31c19c331aaa2dbd9bd2b2e2655de5766a9d Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,48 @@
From b2740b7e4a03bb8331d48b54b119afea76bb9d5f Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,65 @@
From afda8b60ba0712abe01ae1e64c5f7a067a0e6492 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,86 @@
From c140a086838e7c9af87842036f891b8393a8c4bc Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,54 @@
From 630deb8c0d8b02b670ced4b7030414bcf17aa080 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <daniel.kiper@oracle.com>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,85 @@
From 50a11a81bc842c58962244a2dc86bbd31a426e12 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,91 @@
From 6d2668dea3774ed74c4cd1eadd146f1b846bc3d4 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,75 @@
From fcd7aa0c278f7cf3fb9f93f1a3966e1792339eb6 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,36 @@
From dd539d695482069d28b40f2d3821f710cdcf6ee6 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -0,0 +1,55 @@
From da90d62316a3b105d2fbd7334d6521936bd6dcf6 Mon Sep 17 00:00:00 2001
From: Zhang Boyang <zhangboyang.id@gmail.com>
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 <dja@axtens.net>
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
---
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

View File

@ -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