From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Daniel Axtens 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 Reviewed-by: Daniel Kiper (cherry picked from commit 0e1d163382669bd734439d8864ee969616d971d9) [rharwood: context conflict] Signed-off-by: Robbie Harwood (cherry picked from commit 4c631c8119206b3178912df2905434d967661c3d) (cherry picked from commit 6d5d5f51266b8113c6ba560835500e3c135f3722) (cherry picked from commit b134798cc28f7989e729d8c3fc83734444cdd00a) --- 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 71bb76f9c4..dcfdaa8596 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. */