Merge "grub2/grub-efi: fix CVE-2022-2601/CVE-2022-3775"
This commit is contained in:
commit
2f3c29986e
@ -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. */
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -17,3 +17,18 @@
|
|||||||
0017-grub-shim-verify-Report-that-the-loaded-object-is-ve.patch
|
0017-grub-shim-verify-Report-that-the-loaded-object-is-ve.patch
|
||||||
0018-grub-verify-Add-strict_security-variable.patch
|
0018-grub-verify-Add-strict_security-variable.patch
|
||||||
0019-Disable-inside-lockdown-and-shim_lock-verifiers.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
|
||||||
|
@ -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. */
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -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
|
||||||
|
|
@ -1,2 +1,17 @@
|
|||||||
0001-grub2-add-tboot.patch
|
0001-grub2-add-tboot.patch
|
||||||
0002-grub2-checking-if-loop-devices-are-available.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
|
||||||
|
Loading…
Reference in New Issue
Block a user