ed1787d
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
ed1787d
From: Zhang Boyang <zhangboyang.id@gmail.com>
ed1787d
Date: Mon, 24 Oct 2022 07:15:41 +0800
ed1787d
Subject: [PATCH] font: Harden grub_font_blit_glyph() and
ed1787d
 grub_font_blit_glyph_mirror()
ed1787d
ed1787d
As a mitigation and hardening measure add sanity checks to
ed1787d
grub_font_blit_glyph() and grub_font_blit_glyph_mirror(). This patch
ed1787d
makes these two functions do nothing if target blitting area isn't fully
ed1787d
contained in target bitmap. Therefore, if complex calculations in caller
ed1787d
overflows and malicious coordinates are given, we are still safe because
ed1787d
any coordinates which result in out-of-bound-write are rejected. However,
ed1787d
this patch only checks for invalid coordinates, and doesn't provide any
ed1787d
protection against invalid source glyph or destination glyph, e.g.
ed1787d
mismatch between glyph size and buffer size.
ed1787d
ed1787d
This hardening measure is designed to mitigate possible overflows in
ed1787d
blit_comb(). If overflow occurs, it may return invalid bounding box
ed1787d
during dry run and call grub_font_blit_glyph() with malicious
ed1787d
coordinates during actual blitting. However, we are still safe because
ed1787d
the scratch glyph itself is valid, although its size makes no sense, and
ed1787d
any invalid coordinates are rejected.
ed1787d
ed1787d
It would be better to call grub_fatal() if illegal parameter is detected.
ed1787d
However, doing this may end up in a dangerous recursion because grub_fatal()
ed1787d
would print messages to the screen and we are in the progress of drawing
ed1787d
characters on the screen.
ed1787d
ed1787d
Reported-by: Daniel Axtens <dja@axtens.net>
ed1787d
Signed-off-by: Zhang Boyang <zhangboyang.id@gmail.com>
ed1787d
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
ed1787d
(cherry picked from commit fcd7aa0c278f7cf3fb9f93f1a3966e1792339eb6)
ed1787d
---
ed1787d
 grub-core/font/font.c | 14 ++++++++++++++
ed1787d
 1 file changed, 14 insertions(+)
ed1787d
ed1787d
diff --git a/grub-core/font/font.c b/grub-core/font/font.c
ed1787d
index 12a5f0d08c..29fbb94294 100644
ed1787d
--- a/grub-core/font/font.c
ed1787d
+++ b/grub-core/font/font.c
ed1787d
@@ -1069,8 +1069,15 @@ static void
ed1787d
 grub_font_blit_glyph (struct grub_font_glyph *target,
ed1787d
 		      struct grub_font_glyph *src, unsigned dx, unsigned dy)
ed1787d
 {
ed1787d
+  grub_uint16_t max_x, max_y;
ed1787d
   unsigned src_bit, tgt_bit, src_byte, tgt_byte;
ed1787d
   unsigned i, j;
ed1787d
+
ed1787d
+  /* Harden against out-of-bound writes. */
ed1787d
+  if ((grub_add (dx, src->width, &max_x) || max_x > target->width) ||
ed1787d
+      (grub_add (dy, src->height, &max_y) || max_y > target->height))
ed1787d
+    return;
ed1787d
+
ed1787d
   for (i = 0; i < src->height; i++)
ed1787d
     {
ed1787d
       src_bit = (src->width * i) % 8;
ed1787d
@@ -1102,9 +1109,16 @@ grub_font_blit_glyph_mirror (struct grub_font_glyph *target,
ed1787d
 			     struct grub_font_glyph *src,
ed1787d
 			     unsigned dx, unsigned dy)
ed1787d
 {
ed1787d
+  grub_uint16_t max_x, max_y;
ed1787d
   unsigned tgt_bit, src_byte, tgt_byte;
ed1787d
   signed src_bit;
ed1787d
   unsigned i, j;
ed1787d
+
ed1787d
+  /* Harden against out-of-bound writes. */
ed1787d
+  if ((grub_add (dx, src->width, &max_x) || max_x > target->width) ||
ed1787d
+      (grub_add (dy, src->height, &max_y) || max_y > target->height))
ed1787d
+    return;
ed1787d
+
ed1787d
   for (i = 0; i < src->height; i++)
ed1787d
     {
ed1787d
       src_bit = (src->width * i + src->width - 1) % 8;