Blob Blame History Raw
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Daniel Axtens <dja@axtens.net>
Date: Tue, 6 Jul 2021 18:51:35 +1000
Subject: [PATCH] video/readers/png: Drop greyscale support to fix heap
 out-of-bounds write

A 16-bit greyscale PNG without alpha is processed in the following loop:

      for (i = 0; i < (data->image_width * data->image_height);
	   i++, d1 += 4, d2 += 2)
	{
	  d1[R3] = d2[1];
	  d1[G3] = d2[1];
	  d1[B3] = d2[1];
	}

The increment of d1 is wrong. d1 is incremented by 4 bytes per iteration,
but there are only 3 bytes allocated for storage. This means that image
data will overwrite somewhat-attacker-controlled parts of memory - 3 bytes
out of every 4 following the end of the image.

This has existed since greyscale support was added in 2013 in commit
3ccf16dff98f (grub-core/video/readers/png.c: Support grayscale).

Saving starfield.png as a 16-bit greyscale image without alpha in the gimp
and attempting to load it causes grub-emu to crash - I don't think this code
has ever worked.

Delete all PNG greyscale support.

Fixes: CVE-2021-3695

Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
(cherry picked from commit 0e1d163382669bd734439d8864ee969616d971d9)
[rharwood: context conflict]
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
(cherry picked from commit 4c631c8119206b3178912df2905434d967661c3d)
(cherry picked from commit 6d5d5f51266b8113c6ba560835500e3c135f3722)
(cherry picked from commit b20fc5589561a8c57a2071b2ae93fcdcf51a10d4)
---
 grub-core/video/readers/png.c | 85 +++----------------------------------------
 1 file changed, 6 insertions(+), 79 deletions(-)

diff --git a/grub-core/video/readers/png.c b/grub-core/video/readers/png.c
index 8955b8ecfd..a3161e25b6 100644
--- a/grub-core/video/readers/png.c
+++ b/grub-core/video/readers/png.c
@@ -100,7 +100,7 @@ struct grub_png_data
 
   unsigned image_width, image_height;
   int bpp, is_16bit;
-  int raw_bytes, is_gray, is_alpha, is_palette;
+  int raw_bytes, is_alpha, is_palette;
   int row_bytes, color_bits;
   grub_uint8_t *image_data;
 
@@ -296,13 +296,13 @@ grub_png_decode_image_header (struct grub_png_data *data)
     data->bpp = 3;
   else
     {
-      data->is_gray = 1;
-      data->bpp = 1;
+      return grub_error (GRUB_ERR_BAD_FILE_TYPE,
+			 "png: color type not supported");
     }
 
   if ((color_bits != 8) && (color_bits != 16)
       && (color_bits != 4
-	  || !(data->is_gray || data->is_palette)))
+	  || !data->is_palette))
     return grub_error (GRUB_ERR_BAD_FILE_TYPE,
                        "png: bit depth must be 8 or 16");
 
@@ -331,7 +331,7 @@ grub_png_decode_image_header (struct grub_png_data *data)
     }
 
 #ifndef GRUB_CPU_WORDS_BIGENDIAN
-  if (data->is_16bit || data->is_gray || data->is_palette)
+  if (data->is_16bit || data->is_palette)
 #endif
     {
       data->image_data = grub_calloc (data->image_height, data->row_bytes);
@@ -899,27 +899,8 @@ grub_png_convert_image (struct grub_png_data *data)
       int shift;
       int mask = (1 << data->color_bits) - 1;
       unsigned j;
-      if (data->is_gray)
-	{
-	  /* Generic formula is
-	     (0xff * i) / ((1U << data->color_bits) - 1)
-	     but for allowed bit depth of 1, 2 and for it's
-	     equivalent to
-	     (0xff / ((1U << data->color_bits) - 1)) * i
-	     Precompute the multipliers to avoid division.
-	  */
 
-	  const grub_uint8_t multipliers[5] = { 0xff, 0xff, 0x55, 0x24, 0x11 };
-	  for (i = 0; i < (1U << data->color_bits); i++)
-	    {
-	      grub_uint8_t col = multipliers[data->color_bits] * i;
-	      palette[i][0] = col;
-	      palette[i][1] = col;
-	      palette[i][2] = col;
-	    }
-	}
-      else
-	grub_memcpy (palette, data->palette, 3 << data->color_bits);
+      grub_memcpy (palette, data->palette, 3 << data->color_bits);
       d1c = d1;
       d2c = d2;
       for (j = 0; j < data->image_height; j++, d1c += data->image_width * 3,
@@ -956,60 +937,6 @@ grub_png_convert_image (struct grub_png_data *data)
 	}
       return;
     }
-  
-  if (data->is_gray)
-    {
-      switch (data->bpp)
-	{
-	case 4:
-	  /* 16-bit gray with alpha.  */
-	  for (i = 0; i < (data->image_width * data->image_height);
-	       i++, d1 += 4, d2 += 4)
-	    {
-	      d1[R4] = d2[3];
-	      d1[G4] = d2[3];
-	      d1[B4] = d2[3];
-	      d1[A4] = d2[1];
-	    }
-	  break;
-	case 2:
-	  if (data->is_16bit)
-	    /* 16-bit gray without alpha.  */
-	    {
-	      for (i = 0; i < (data->image_width * data->image_height);
-		   i++, d1 += 4, d2 += 2)
-		{
-		  d1[R3] = d2[1];
-		  d1[G3] = d2[1];
-		  d1[B3] = d2[1];
-		}
-	    }
-	  else
-	    /* 8-bit gray with alpha.  */
-	    {
-	      for (i = 0; i < (data->image_width * data->image_height);
-		   i++, d1 += 4, d2 += 2)
-		{
-		  d1[R4] = d2[1];
-		  d1[G4] = d2[1];
-		  d1[B4] = d2[1];
-		  d1[A4] = d2[0];
-		}
-	    }
-	  break;
-	  /* 8-bit gray without alpha.  */
-	case 1:
-	  for (i = 0; i < (data->image_width * data->image_height);
-	       i++, d1 += 3, d2++)
-	    {
-	      d1[R3] = d2[0];
-	      d1[G3] = d2[0];
-	      d1[B3] = d2[0];
-	    }
-	  break;
-	}
-      return;
-    }
 
     {
   /* Only copy the upper 8 bit.  */