From 1066336dc88ef59556be91003388d8a1dfd0fd91 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 7 Sep 2020 17:27:36 +0200 Subject: [PATCH] luks: Fix out-of-bounds copy of UUID When configuring a LUKS disk, we copy over the UUID from the LUKS header into the new grub_cryptodisk_t structure via grub_memcpy(). As size we mistakenly use the size of the grub_cryptodisk_t UUID field, which is guaranteed to be strictly bigger than the LUKS UUID field we're copying. As a result, the copy always goes out-of-bounds and copies some garbage from other surrounding fields. During runtime, this isn't noticed due to the fact that we always NUL-terminate the UUID and thus never hit the trailing garbage. Fix the issue by using the size of the local stripped UUID field. Signed-off-by: Patrick Steinhardt Reviewed-by: Daniel Kiper --- grub-core/disk/luks.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c index 6ae162601..59702067a 100644 --- a/grub-core/disk/luks.c +++ b/grub-core/disk/luks.c @@ -95,6 +95,7 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid, || grub_be_to_cpu16 (header.version) != 1) return NULL; + grub_memset (uuid, 0, sizeof (uuid)); optr = uuid; for (iptr = header.uuid; iptr < &header.uuid[ARRAY_SIZE (header.uuid)]; iptr++) @@ -125,7 +126,7 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid, newdev->source_disk = NULL; newdev->log_sector_size = 9; newdev->total_length = grub_disk_get_size (disk) - newdev->offset; - grub_memcpy (newdev->uuid, uuid, sizeof (newdev->uuid)); + grub_memcpy (newdev->uuid, uuid, sizeof (uuid)); newdev->modname = "luks"; /* Configure the hash used for the AF splitter and HMAC. */