]> git.proxmox.com Git - grub2.git/commitdiff
font: Fix size overflow in grub_font_get_glyph_internal()
authorZhang Boyang <zhangboyang.id@gmail.com>
Thu, 4 Aug 2022 16:51:20 +0000 (00:51 +0800)
committerSteve McIntyre <93sam@debian.org>
Sat, 12 Nov 2022 22:51:49 +0000 (22:51 +0000)
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>
debian/patches/cve_2022_2601/0003-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch [new file with mode: 0644]
grub-core/font/font.c
include/grub/bitmap.h
include/grub/safemath.h

diff --git a/debian/patches/cve_2022_2601/0003-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch b/debian/patches/cve_2022_2601/0003-font-Fix-size-overflow-in-grub_font_get_glyph_intern.patch
new file mode 100644 (file)
index 0000000..46ccb00
--- /dev/null
@@ -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
index 2f09a4a55b4955a0a49034cf885e45a070760613..6a3fbebbd8f630ccfd71e034627e504bc20a3b2b 100644 (file)
@@ -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;
index 149d37bfe6350600d8b5d1e72aae3d0204380f0b..431048936132f2cd9c9a1a57ec648c1cc20ab8e2 100644 (file)
@@ -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);
 
index c17b89bba17b53bf7f3d55375e4590a679d25523..bb0f826de18ada82d9f44ce6612b221b6e60eb77 100644 (file)
@@ -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