|
|
5975ab |
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
|
a4d572 |
From: Peter Jones <pjones@redhat.com>
|
|
|
a4d572 |
Date: Sun, 19 Jul 2020 16:53:27 -0400
|
|
|
5975ab |
Subject: [PATCH] efi: fix some malformed device path arithmetic errors.
|
|
|
a4d572 |
|
|
|
a4d572 |
Several places we take the length of a device path and subtract 4 from
|
|
|
a4d572 |
it, without ever checking that it's >= 4. There are also cases where
|
|
|
a4d572 |
this kind of malformation will result in unpredictable iteration,
|
|
|
a4d572 |
including treating the length from one dp node as the type in the next
|
|
|
a4d572 |
node. These are all errors, no matter where the data comes from.
|
|
|
a4d572 |
|
|
|
a4d572 |
This patch adds a checking macro, GRUB_EFI_DEVICE_PATH_VALID(), which
|
|
|
a4d572 |
can be used in several places, and makes GRUB_EFI_NEXT_DEVICE_PATH()
|
|
|
a4d572 |
return NULL and GRUB_EFI_END_ENTIRE_DEVICE_PATH() evaluate as true when
|
|
|
a4d572 |
the length is too small. Additionally, it makes several places in the
|
|
|
a4d572 |
code check for and return errors in these cases.
|
|
|
a4d572 |
|
|
|
a4d572 |
Signed-off-by: Peter Jones <pjones@redhat.com>
|
|
|
a4d572 |
Upstream-commit-id: 23e68a83990
|
|
|
a4d572 |
---
|
|
|
5975ab |
grub-core/kern/efi/efi.c | 67 ++++++++++++++++++++++++++++++++------
|
|
|
5975ab |
grub-core/loader/efi/chainloader.c | 19 +++++++++--
|
|
|
5975ab |
grub-core/loader/i386/xnu.c | 9 ++---
|
|
|
5975ab |
include/grub/efi/api.h | 14 +++++---
|
|
|
a4d572 |
4 files changed, 88 insertions(+), 21 deletions(-)
|
|
|
a4d572 |
|
|
|
a4d572 |
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
|
|
|
a4d572 |
index b1379b92fb8..03de9cb14e7 100644
|
|
|
a4d572 |
--- a/grub-core/kern/efi/efi.c
|
|
|
a4d572 |
+++ b/grub-core/kern/efi/efi.c
|
|
|
a4d572 |
@@ -344,7 +344,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
|
|
|
a4d572 |
|
|
|
a4d572 |
dp = dp0;
|
|
|
a4d572 |
|
|
|
a4d572 |
- while (1)
|
|
|
a4d572 |
+ while (dp)
|
|
|
a4d572 |
{
|
|
|
a4d572 |
grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp);
|
|
|
a4d572 |
grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp);
|
|
|
a4d572 |
@@ -354,9 +354,15 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
|
|
|
a4d572 |
if (type == GRUB_EFI_MEDIA_DEVICE_PATH_TYPE
|
|
|
a4d572 |
&& subtype == GRUB_EFI_FILE_PATH_DEVICE_PATH_SUBTYPE)
|
|
|
a4d572 |
{
|
|
|
a4d572 |
- grub_efi_uint16_t len;
|
|
|
a4d572 |
- len = ((GRUB_EFI_DEVICE_PATH_LENGTH (dp) - 4)
|
|
|
a4d572 |
- / sizeof (grub_efi_char16_t));
|
|
|
a4d572 |
+ grub_efi_uint16_t len = GRUB_EFI_DEVICE_PATH_LENGTH (dp);
|
|
|
a4d572 |
+
|
|
|
a4d572 |
+ if (len < 4)
|
|
|
a4d572 |
+ {
|
|
|
a4d572 |
+ grub_error (GRUB_ERR_OUT_OF_RANGE,
|
|
|
a4d572 |
+ "malformed EFI Device Path node has length=%d", len);
|
|
|
a4d572 |
+ return NULL;
|
|
|
a4d572 |
+ }
|
|
|
a4d572 |
+ len = (len - 4) / sizeof (grub_efi_char16_t);
|
|
|
a4d572 |
filesize += GRUB_MAX_UTF8_PER_UTF16 * len + 2;
|
|
|
a4d572 |
}
|
|
|
a4d572 |
|
|
|
a4d572 |
@@ -372,7 +378,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
|
|
|
a4d572 |
if (!name)
|
|
|
a4d572 |
return NULL;
|
|
|
a4d572 |
|
|
|
a4d572 |
- while (1)
|
|
|
a4d572 |
+ while (dp)
|
|
|
a4d572 |
{
|
|
|
a4d572 |
grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp);
|
|
|
a4d572 |
grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp);
|
|
|
a4d572 |
@@ -388,8 +394,15 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0)
|
|
|
a4d572 |
|
|
|
a4d572 |
*p++ = '/';
|
|
|
a4d572 |
|
|
|
a4d572 |
- len = ((GRUB_EFI_DEVICE_PATH_LENGTH (dp) - 4)
|
|
|
a4d572 |
- / sizeof (grub_efi_char16_t));
|
|
|
a4d572 |
+ len = GRUB_EFI_DEVICE_PATH_LENGTH (dp);
|
|
|
a4d572 |
+ if (len < 4)
|
|
|
a4d572 |
+ {
|
|
|
a4d572 |
+ grub_error (GRUB_ERR_OUT_OF_RANGE,
|
|
|
a4d572 |
+ "malformed EFI Device Path node has length=%d", len);
|
|
|
a4d572 |
+ return NULL;
|
|
|
a4d572 |
+ }
|
|
|
a4d572 |
+
|
|
|
a4d572 |
+ len = (len - 4) / sizeof (grub_efi_char16_t);
|
|
|
a4d572 |
fp = (grub_efi_file_path_device_path_t *) dp;
|
|
|
a4d572 |
/* According to EFI spec Path Name is NULL terminated */
|
|
|
a4d572 |
while (len > 0 && fp->path_name[len - 1] == 0)
|
|
|
a4d572 |
@@ -464,7 +477,26 @@ grub_efi_duplicate_device_path (const grub_efi_device_path_t *dp)
|
|
|
a4d572 |
;
|
|
|
a4d572 |
p = GRUB_EFI_NEXT_DEVICE_PATH (p))
|
|
|
a4d572 |
{
|
|
|
a4d572 |
- total_size += GRUB_EFI_DEVICE_PATH_LENGTH (p);
|
|
|
a4d572 |
+ grub_size_t len = GRUB_EFI_DEVICE_PATH_LENGTH (p);
|
|
|
a4d572 |
+
|
|
|
a4d572 |
+ /*
|
|
|
a4d572 |
+ * In the event that we find a node that's completely garbage, for
|
|
|
a4d572 |
+ * example if we get to 0x7f 0x01 0x02 0x00 ... (EndInstance with a size
|
|
|
a4d572 |
+ * of 2), GRUB_EFI_END_ENTIRE_DEVICE_PATH() will be true and
|
|
|
a4d572 |
+ * GRUB_EFI_NEXT_DEVICE_PATH() will return NULL, so we won't continue,
|
|
|
a4d572 |
+ * and neither should our consumers, but there won't be any error raised
|
|
|
a4d572 |
+ * even though the device path is junk.
|
|
|
a4d572 |
+ *
|
|
|
a4d572 |
+ * This keeps us from passing junk down back to our caller.
|
|
|
a4d572 |
+ */
|
|
|
a4d572 |
+ if (len < 4)
|
|
|
a4d572 |
+ {
|
|
|
a4d572 |
+ grub_error (GRUB_ERR_OUT_OF_RANGE,
|
|
|
a4d572 |
+ "malformed EFI Device Path node has length=%d", len);
|
|
|
a4d572 |
+ return NULL;
|
|
|
a4d572 |
+ }
|
|
|
a4d572 |
+
|
|
|
a4d572 |
+ total_size += len;
|
|
|
a4d572 |
if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (p))
|
|
|
a4d572 |
break;
|
|
|
a4d572 |
}
|
|
|
a4d572 |
@@ -509,7 +541,7 @@ dump_vendor_path (const char *type, grub_efi_vendor_device_path_t *vendor)
|
|
|
a4d572 |
void
|
|
|
a4d572 |
grub_efi_print_device_path (grub_efi_device_path_t *dp)
|
|
|
a4d572 |
{
|
|
|
a4d572 |
- while (1)
|
|
|
a4d572 |
+ while (GRUB_EFI_DEVICE_PATH_VALID (dp))
|
|
|
a4d572 |
{
|
|
|
a4d572 |
grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp);
|
|
|
a4d572 |
grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp);
|
|
|
a4d572 |
@@ -981,7 +1013,11 @@ grub_efi_compare_device_paths (const grub_efi_device_path_t *dp1,
|
|
|
a4d572 |
/* Return non-zero. */
|
|
|
a4d572 |
return 1;
|
|
|
a4d572 |
|
|
|
a4d572 |
- while (1)
|
|
|
a4d572 |
+ if (dp1 == dp2)
|
|
|
a4d572 |
+ return 0;
|
|
|
a4d572 |
+
|
|
|
a4d572 |
+ while (GRUB_EFI_DEVICE_PATH_VALID (dp1)
|
|
|
a4d572 |
+ && GRUB_EFI_DEVICE_PATH_VALID (dp2))
|
|
|
a4d572 |
{
|
|
|
a4d572 |
grub_efi_uint8_t type1, type2;
|
|
|
a4d572 |
grub_efi_uint8_t subtype1, subtype2;
|
|
|
a4d572 |
@@ -1017,5 +1053,16 @@ grub_efi_compare_device_paths (const grub_efi_device_path_t *dp1,
|
|
|
a4d572 |
dp2 = (grub_efi_device_path_t *) ((char *) dp2 + len2);
|
|
|
a4d572 |
}
|
|
|
a4d572 |
|
|
|
a4d572 |
+ /*
|
|
|
a4d572 |
+ * There's no "right" answer here, but we probably don't want to call a valid
|
|
|
a4d572 |
+ * dp and an invalid dp equal, so pick one way or the other.
|
|
|
a4d572 |
+ */
|
|
|
a4d572 |
+ if (GRUB_EFI_DEVICE_PATH_VALID (dp1) &&
|
|
|
a4d572 |
+ !GRUB_EFI_DEVICE_PATH_VALID (dp2))
|
|
|
a4d572 |
+ return 1;
|
|
|
a4d572 |
+ else if (!GRUB_EFI_DEVICE_PATH_VALID (dp1) &&
|
|
|
a4d572 |
+ GRUB_EFI_DEVICE_PATH_VALID (dp2))
|
|
|
a4d572 |
+ return -1;
|
|
|
a4d572 |
+
|
|
|
a4d572 |
return 0;
|
|
|
a4d572 |
}
|
|
|
a4d572 |
diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c
|
|
|
a4d572 |
index 2da119ad513..c2411b6dab2 100644
|
|
|
a4d572 |
--- a/grub-core/loader/efi/chainloader.c
|
|
|
a4d572 |
+++ b/grub-core/loader/efi/chainloader.c
|
|
|
a4d572 |
@@ -125,6 +125,12 @@ copy_file_path (grub_efi_file_path_device_path_t *fp,
|
|
|
a4d572 |
fp->header.type = GRUB_EFI_MEDIA_DEVICE_PATH_TYPE;
|
|
|
a4d572 |
fp->header.subtype = GRUB_EFI_FILE_PATH_DEVICE_PATH_SUBTYPE;
|
|
|
a4d572 |
|
|
|
a4d572 |
+ if (!GRUB_EFI_DEVICE_PATH_VALID ((grub_efi_device_path_t *)fp))
|
|
|
a4d572 |
+ {
|
|
|
a4d572 |
+ grub_error (GRUB_ERR_BAD_ARGUMENT, "EFI Device Path is invalid");
|
|
|
a4d572 |
+ return;
|
|
|
a4d572 |
+ }
|
|
|
a4d572 |
+
|
|
|
a4d572 |
path_name = grub_calloc (len, GRUB_MAX_UTF16_PER_UTF8 * sizeof (*path_name));
|
|
|
a4d572 |
if (!path_name)
|
|
|
a4d572 |
return;
|
|
|
a4d572 |
@@ -164,9 +170,18 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename)
|
|
|
a4d572 |
|
|
|
a4d572 |
size = 0;
|
|
|
a4d572 |
d = dp;
|
|
|
a4d572 |
- while (1)
|
|
|
a4d572 |
+ while (d)
|
|
|
a4d572 |
{
|
|
|
a4d572 |
- size += GRUB_EFI_DEVICE_PATH_LENGTH (d);
|
|
|
a4d572 |
+ grub_size_t len = GRUB_EFI_DEVICE_PATH_LENGTH (d);
|
|
|
a4d572 |
+
|
|
|
a4d572 |
+ if (len < 4)
|
|
|
a4d572 |
+ {
|
|
|
a4d572 |
+ grub_error (GRUB_ERR_OUT_OF_RANGE,
|
|
|
a4d572 |
+ "malformed EFI Device Path node has length=%d", len);
|
|
|
a4d572 |
+ return NULL;
|
|
|
a4d572 |
+ }
|
|
|
a4d572 |
+
|
|
|
a4d572 |
+ size += len;
|
|
|
a4d572 |
if ((GRUB_EFI_END_ENTIRE_DEVICE_PATH (d)))
|
|
|
a4d572 |
break;
|
|
|
a4d572 |
d = GRUB_EFI_NEXT_DEVICE_PATH (d);
|
|
|
a4d572 |
diff --git a/grub-core/loader/i386/xnu.c b/grub-core/loader/i386/xnu.c
|
|
|
a4d572 |
index c760db30fc0..44f7ebfa2b6 100644
|
|
|
a4d572 |
--- a/grub-core/loader/i386/xnu.c
|
|
|
a4d572 |
+++ b/grub-core/loader/i386/xnu.c
|
|
|
a4d572 |
@@ -515,14 +515,15 @@ grub_cmd_devprop_load (grub_command_t cmd __attribute__ ((unused)),
|
|
|
a4d572 |
|
|
|
a4d572 |
devhead = buf;
|
|
|
a4d572 |
buf = devhead + 1;
|
|
|
a4d572 |
- dpstart = buf;
|
|
|
a4d572 |
+ dp = dpstart = buf;
|
|
|
a4d572 |
|
|
|
a4d572 |
- do
|
|
|
a4d572 |
+ while (GRUB_EFI_DEVICE_PATH_VALID (dp) && buf < bufend)
|
|
|
a4d572 |
{
|
|
|
a4d572 |
- dp = buf;
|
|
|
a4d572 |
buf = (char *) buf + GRUB_EFI_DEVICE_PATH_LENGTH (dp);
|
|
|
a4d572 |
+ if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp))
|
|
|
a4d572 |
+ break;
|
|
|
a4d572 |
+ dp = buf;
|
|
|
a4d572 |
}
|
|
|
a4d572 |
- while (!GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp) && buf < bufend);
|
|
|
a4d572 |
|
|
|
a4d572 |
dev = grub_xnu_devprop_add_device (dpstart, (char *) buf
|
|
|
a4d572 |
- (char *) dpstart);
|
|
|
a4d572 |
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
|
|
|
a4d572 |
index 6c440c61316..a092fddb629 100644
|
|
|
a4d572 |
--- a/include/grub/efi/api.h
|
|
|
a4d572 |
+++ b/include/grub/efi/api.h
|
|
|
a4d572 |
@@ -671,6 +671,7 @@ typedef struct grub_efi_device_path grub_efi_device_path_protocol_t;
|
|
|
a4d572 |
#define GRUB_EFI_DEVICE_PATH_TYPE(dp) ((dp)->type & 0x7f)
|
|
|
a4d572 |
#define GRUB_EFI_DEVICE_PATH_SUBTYPE(dp) ((dp)->subtype)
|
|
|
a4d572 |
#define GRUB_EFI_DEVICE_PATH_LENGTH(dp) ((dp)->length)
|
|
|
a4d572 |
+#define GRUB_EFI_DEVICE_PATH_VALID(dp) ((dp) != NULL && GRUB_EFI_DEVICE_PATH_LENGTH (dp) >= 4)
|
|
|
a4d572 |
|
|
|
a4d572 |
/* The End of Device Path nodes. */
|
|
|
a4d572 |
#define GRUB_EFI_END_DEVICE_PATH_TYPE (0xff & 0x7f)
|
|
|
a4d572 |
@@ -679,13 +680,16 @@ typedef struct grub_efi_device_path grub_efi_device_path_protocol_t;
|
|
|
a4d572 |
#define GRUB_EFI_END_THIS_DEVICE_PATH_SUBTYPE 0x01
|
|
|
a4d572 |
|
|
|
a4d572 |
#define GRUB_EFI_END_ENTIRE_DEVICE_PATH(dp) \
|
|
|
a4d572 |
- (GRUB_EFI_DEVICE_PATH_TYPE (dp) == GRUB_EFI_END_DEVICE_PATH_TYPE \
|
|
|
a4d572 |
- && (GRUB_EFI_DEVICE_PATH_SUBTYPE (dp) \
|
|
|
a4d572 |
- == GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE))
|
|
|
a4d572 |
+ (!GRUB_EFI_DEVICE_PATH_VALID (dp) || \
|
|
|
a4d572 |
+ (GRUB_EFI_DEVICE_PATH_TYPE (dp) == GRUB_EFI_END_DEVICE_PATH_TYPE \
|
|
|
a4d572 |
+ && (GRUB_EFI_DEVICE_PATH_SUBTYPE (dp) \
|
|
|
a4d572 |
+ == GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE)))
|
|
|
a4d572 |
|
|
|
a4d572 |
#define GRUB_EFI_NEXT_DEVICE_PATH(dp) \
|
|
|
a4d572 |
- ((grub_efi_device_path_t *) ((char *) (dp) \
|
|
|
a4d572 |
- + GRUB_EFI_DEVICE_PATH_LENGTH (dp)))
|
|
|
a4d572 |
+ (GRUB_EFI_DEVICE_PATH_VALID (dp) \
|
|
|
a4d572 |
+ ? ((grub_efi_device_path_t *) \
|
|
|
a4d572 |
+ ((char *) (dp) + GRUB_EFI_DEVICE_PATH_LENGTH (dp))) \
|
|
|
a4d572 |
+ : NULL)
|
|
|
a4d572 |
|
|
|
a4d572 |
/* Hardware Device Path. */
|
|
|
a4d572 |
#define GRUB_EFI_HARDWARE_DEVICE_PATH_TYPE 1
|