Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package grub2 for openSUSE:Factory checked in at 2022-11-16 15:42:37 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/grub2 (Old) and /work/SRC/openSUSE:Factory/.grub2.new.1597 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "grub2" Wed Nov 16 15:42:37 2022 rev:274 rq:1035937 version:2.06 Changes: -------- --- /work/SRC/openSUSE:Factory/grub2/grub2.changes 2022-11-14 14:28:46.594904018 +0100 +++ /work/SRC/openSUSE:Factory/.grub2.new.1597/grub2.changes 2022-11-16 15:42:44.979698229 +0100 @@ -1,0 +2,20 @@ +Wed Nov 16 02:36:23 UTC 2022 - Michael Chang <mch...@suse.com> + +- Security fixes and hardenings + * 0001-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch + * 0002-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch +- Fix CVE-2022-2601 (bsc#1205178) + * 0003-font-Fix-several-integer-overflows-in-grub_font_cons.patch + * 0004-font-Remove-grub_font_dup_glyph.patch + * 0005-font-Fix-integer-overflow-in-ensure_comb_space.patch + * 0006-font-Fix-integer-overflow-in-BMP-index.patch + * 0007-font-Fix-integer-underflow-in-binary-search-of-char-.patch + * 0008-fbutil-Fix-integer-overflow.patch +- Fix CVE-2022-3775 (bsc#1205182) + * 0009-font-Fix-an-integer-underflow-in-blit_comb.patch + * 0010-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch + * 0011-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch + * 0012-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch +- Bump upstream SBAT generation to 3 + +------------------------------------------------------------------- New: ---- 0001-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch 0002-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch 0003-font-Fix-several-integer-overflows-in-grub_font_cons.patch 0004-font-Remove-grub_font_dup_glyph.patch 0005-font-Fix-integer-overflow-in-ensure_comb_space.patch 0006-font-Fix-integer-overflow-in-BMP-index.patch 0007-font-Fix-integer-underflow-in-binary-search-of-char-.patch 0008-fbutil-Fix-integer-overflow.patch 0009-font-Fix-an-integer-underflow-in-blit_comb.patch 0010-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch 0011-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch 0012-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ grub2.spec ++++++ --- /var/tmp/diff_new_pack.19jcUF/_old 2022-11-16 15:42:48.899712440 +0100 +++ /var/tmp/diff_new_pack.19jcUF/_new 2022-11-16 15:42:48.903712455 +0100 @@ -22,7 +22,7 @@ %if %{defined sbat_distro} # SBAT metadata %define sbat_generation 1 -%define sbat_generation_grub 2 +%define sbat_generation_grub 3 %else %{error please define sbat_distro, sbat_distro_summary and sbat_distro_url} %endif @@ -465,6 +465,18 @@ # (PED-1990) GRUB2: Measure the kernel on POWER10 and extend TPM PCRs Patch936: 0001-ibmvtpm-Add-support-for-trusted-boot-using-a-vTPM-2..patch Patch937: 0002-ieee1275-implement-vec5-for-cas-negotiation.patch +Patch938: 0001-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch +Patch939: 0002-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch +Patch940: 0003-font-Fix-several-integer-overflows-in-grub_font_cons.patch +Patch941: 0004-font-Remove-grub_font_dup_glyph.patch +Patch942: 0005-font-Fix-integer-overflow-in-ensure_comb_space.patch +Patch943: 0006-font-Fix-integer-overflow-in-BMP-index.patch +Patch944: 0007-font-Fix-integer-underflow-in-binary-search-of-char-.patch +Patch945: 0008-fbutil-Fix-integer-overflow.patch +Patch946: 0009-font-Fix-an-integer-underflow-in-blit_comb.patch +Patch947: 0010-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch +Patch948: 0011-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch +Patch949: 0012-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch Requires: gettext-runtime %if 0%{?suse_version} >= 1140 ++++++ 0001-font-Reject-glyphs-exceeds-font-max_glyph_width-or-f.patch ++++++ >From a2606b0cb95f261288c79cafc7295927d868cb04 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Wed, 3 Aug 2022 19:45:33 +0800 Subject: [PATCH 01/12] 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...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 d09bb38d8..2f09a4a55 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.35.3 ++++++ 0002-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch ++++++ >From 84c4323c004b495993a3d0dbfa94d8675ae06f03 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Fri, 5 Aug 2022 00:51:20 +0800 Subject: [PATCH 02/12] 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...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 5728f8ca3..0d9603f61 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> struct grub_video_bitmap { @@ -79,6 +80,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 -- 2.35.3 ++++++ 0003-font-Fix-several-integer-overflows-in-grub_font_cons.patch ++++++ >From a63cbda5bbfa4696c97dc8231e7b81aedaef2fc7 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Fri, 5 Aug 2022 01:58:27 +0800 Subject: [PATCH 03/12] 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...@gmail.com> Signed-off-by: Zhang Boyang <zhangboyang...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 6a3fbebbd..1fa181d4c 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.35.3 ++++++ 0004-font-Remove-grub_font_dup_glyph.patch ++++++ >From 9c99a0d55bf69f25ad41668867f719bbb1828457 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Fri, 5 Aug 2022 02:13:29 +0800 Subject: [PATCH 04/12] 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...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 1fa181d4c..a115a63b0 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.35.3 ++++++ 0005-font-Fix-integer-overflow-in-ensure_comb_space.patch ++++++ >From 2ec7e27b2cac3746f2e658042fd56fe75fee28f2 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Fri, 5 Aug 2022 02:27:05 +0800 Subject: [PATCH 05/12] 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...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 a115a63b0..d0e634040 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.35.3 ++++++ 0006-font-Fix-integer-overflow-in-BMP-index.patch ++++++ >From a97e0ae72604fbd4ebea854ffee127ed59b10f75 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Mon, 15 Aug 2022 02:04:58 +0800 Subject: [PATCH 06/12] 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...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 d0e634040..b208a2871 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.35.3 ++++++ 0007-font-Fix-integer-underflow-in-binary-search-of-char-.patch ++++++ >From 926f1515608e14c0592fc61a8ef37392d7020ca3 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Sun, 14 Aug 2022 18:09:38 +0800 Subject: [PATCH 07/12] 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...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 b208a2871..193dfec04 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.35.3 ++++++ 0008-fbutil-Fix-integer-overflow.patch ++++++ >From 17e9006484e3da9a38a79f5f0f28f18a15fc4cf8 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Tue, 6 Sep 2022 03:03:21 +0800 Subject: [PATCH 08/12] 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...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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.35.3 ++++++ 0009-font-Fix-an-integer-underflow-in-blit_comb.patch ++++++ >From 79924f56f5062c1bae972fd6cd8f38e56980f34d Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Mon, 24 Oct 2022 08:05:35 +0800 Subject: [PATCH 09/12] 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 <d...@axtens.net> Signed-off-by: Zhang Boyang <zhangboyang...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 193dfec04..12a5f0d08 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.35.3 ++++++ 0010-font-Harden-grub_font_blit_glyph-and-grub_font_blit_.patch ++++++ >From f3b30e0d782f36634a9a7ab9d18851b0b7a1bce5 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Mon, 24 Oct 2022 07:15:41 +0800 Subject: [PATCH 10/12] 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 <d...@axtens.net> Signed-off-by: Zhang Boyang <zhangboyang...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 12a5f0d08..29fbb9429 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.35.3 ++++++ 0011-font-Assign-null_font-to-glyphs-in-ascii_font_glyph.patch ++++++ >From bcda6538ffeb516987a1921fbe533aaf8b8c981b Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Fri, 28 Oct 2022 17:29:16 +0800 Subject: [PATCH 11/12] 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 <d...@axtens.net> Signed-off-by: Zhang Boyang <zhangboyang...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 29fbb9429..e6616e610 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.35.3 ++++++ 0012-normal-charset-Fix-an-integer-overflow-in-grub_unico.patch ++++++ >From 5e53d73775f6dc9b9b08536cbac2f8a5e2559903 Mon Sep 17 00:00:00 2001 From: Zhang Boyang <zhangboyang...@gmail.com> Date: Fri, 28 Oct 2022 21:31:39 +0800 Subject: [PATCH 12/12] 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 <d...@axtens.net> Signed-off-by: Zhang Boyang <zhangboyang...@gmail.com> Reviewed-by: Daniel Kiper <daniel.ki...@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 7a5a7c153..c243ca6da 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 4de986a85..c4f6fca04 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.35.3