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