dcavalca / rpms / grub2

Forked from rpms/grub2 3 years ago
Clone

Blame SOURCES/0303-efi-fix-some-malformed-device-path-arithmetic-errors.patch

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