From c3c9cc7adb7d6eeb68086c8244086e115a788542 Mon Sep 17 00:00:00 2001 From: Milan Kyselica Date: Sat, 11 Apr 2026 10:25:19 +0200 Subject: [PATCH] boot: fix integer overflow and division by zero in BMP splash parser Bound image dimensions before computing row_size to prevent overflow in the depth * x multiplication on 32-bit. Without this, crafted dimensions like depth=32 x=0x10000001 wrap to a small row_size that passes all subsequent checks. Reject channel masks where all bits are set (popcount == 32), since 1U << 32 is undefined behavior and causes division by zero on architectures where it evaluates to zero. Move the validation before computing derived values for clarity. Use unsigned 1U in shifts to avoid signed integer overflow UB for popcount == 31. Also reject zero-width and zero-height images. Fixes: https://github.com/systemd/systemd/issues/41589 --- src/boot/splash.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/boot/splash.c b/src/boot/splash.c index e3365b04df..487f212e65 100644 --- a/src/boot/splash.c +++ b/src/boot/splash.c @@ -95,10 +95,17 @@ static EFI_STATUS bmp_parse_header( return EFI_UNSUPPORTED; } - size_t row_size = ((size_t) dib->depth * dib->x + 31) / 32 * 4; - if (file->size - file->offset < dib->y * row_size) + if (dib->x == 0 || dib->y == 0) return EFI_INVALID_PARAMETER; - if (row_size * dib->y > 64 * 1024 * 1024) + + /* Bound dimensions before computing row_size to prevent overflow + * in the (size_t) dib->depth * dib->x multiplication on 32-bit. */ + if (dib->x > (size_t) 64 * 1024 * 1024 / dib->depth || + dib->y > (size_t) 64 * 1024 * 1024 / dib->depth / dib->x) + return EFI_INVALID_PARAMETER; + + size_t row_size = ((size_t) dib->depth * dib->x + 31) / 32 * 4; + if (file->size - file->offset < dib->y * row_size) return EFI_INVALID_PARAMETER; /* check color table */ @@ -145,20 +152,31 @@ static EFI_STATUS read_channel_mask( if (dib->channel_mask_r == 0 || dib->channel_mask_g == 0 || dib->channel_mask_b == 0) return EFI_INVALID_PARAMETER; + /* Reject masks where all bits are set (popcount == 32), since + * 1U << 32 is undefined behavior and causes division by zero + * on architectures where it evaluates to zero. */ + if (popcount(dib->channel_mask_r) >= 32 || + popcount(dib->channel_mask_g) >= 32 || + popcount(dib->channel_mask_b) >= 32) + return EFI_INVALID_PARAMETER; + channel_mask[R] = dib->channel_mask_r; channel_mask[G] = dib->channel_mask_g; channel_mask[B] = dib->channel_mask_b; channel_shift[R] = __builtin_ctz(dib->channel_mask_r); channel_shift[G] = __builtin_ctz(dib->channel_mask_g); channel_shift[B] = __builtin_ctz(dib->channel_mask_b); - channel_scale[R] = 0xff / ((1 << popcount(dib->channel_mask_r)) - 1); - channel_scale[G] = 0xff / ((1 << popcount(dib->channel_mask_g)) - 1); - channel_scale[B] = 0xff / ((1 << popcount(dib->channel_mask_b)) - 1); + channel_scale[R] = 0xff / ((1U << popcount(dib->channel_mask_r)) - 1); + channel_scale[G] = 0xff / ((1U << popcount(dib->channel_mask_g)) - 1); + channel_scale[B] = 0xff / ((1U << popcount(dib->channel_mask_b)) - 1); if (dib->size >= SIZEOF_BMP_DIB_RGBA && dib->channel_mask_a != 0) { + if (popcount(dib->channel_mask_a) >= 32) + return EFI_INVALID_PARAMETER; + channel_mask[A] = dib->channel_mask_a; channel_shift[A] = __builtin_ctz(dib->channel_mask_a); - channel_scale[A] = 0xff / ((1 << popcount(dib->channel_mask_a)) - 1); + channel_scale[A] = 0xff / ((1U << popcount(dib->channel_mask_a)) - 1); } else { channel_mask[A] = 0; channel_shift[A] = 0;