From 1aefeca0f6304a20c1a3711cb9e89c5fdb901b6b Mon Sep 17 00:00:00 2001 From: Zhang Boyang Date: Fri, 5 Aug 2022 00:51:20 +0800 Subject: [PATCH] font: Fix size overflow in grub_font_get_glyph_internal() The length of memory allocation and file read may overflow. This patch fixes the problem by using safemath macros. There is a lot of code repetition like "(x * y + 7) / 8". It is unsafe if overflow happens. This patch introduces grub_video_bitmap_calc_1bpp_bufsz(). It is safe replacement for such code. It has safemath-like prototype. This patch also introduces grub_cast(value, pointer), it casts value to typeof(*pointer) then store the value to *pointer. It returns true when overflow occurs or false if there is no overflow. The semantics of arguments and return value are designed to be consistent with other safemath macros. Signed-off-by: Zhang Boyang Reviewed-by: Daniel Kiper Upstream: 9c76ec09ae08155df27cd237eaea150b4f02f532 [Thomas: needed to backport 768e1ef2fc159f6e14e7246e4be09363708ac39e, which fixes CVE-2022-2601] Signed-off-by: Thomas Petazzoni --- 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 d09bb38d8..876b5b695 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. */ @@ -766,9 +767,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 #include #include +#include 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.41.0