1From c330aa099a38bc5c4d3066954fe35767cc06adb1 Mon Sep 17 00:00:00 2001 2From: Peter Jones <pjones@redhat.com> 3Date: Sun, 19 Jul 2020 16:53:27 -0400 4Subject: [PATCH] efi: Fix some malformed device path arithmetic errors 5MIME-Version: 1.0 6Content-Type: text/plain; charset=UTF-8 7Content-Transfer-Encoding: 8bit 8 9Several places we take the length of a device path and subtract 4 from 10it, without ever checking that it's >= 4. There are also cases where 11this kind of malformation will result in unpredictable iteration, 12including treating the length from one dp node as the type in the next 13node. These are all errors, no matter where the data comes from. 14 15This patch adds a checking macro, GRUB_EFI_DEVICE_PATH_VALID(), which 16can be used in several places, and makes GRUB_EFI_NEXT_DEVICE_PATH() 17return NULL and GRUB_EFI_END_ENTIRE_DEVICE_PATH() evaluate as true when 18the length is too small. Additionally, it makes several places in the 19code check for and return errors in these cases. 20 21Signed-off-by: Peter Jones <pjones@redhat.com> 22Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> 23Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> 24--- 25 grub-core/kern/efi/efi.c | 64 +++++++++++++++++++++++++----- 26 grub-core/loader/efi/chainloader.c | 13 +++++- 27 grub-core/loader/i386/xnu.c | 9 +++-- 28 include/grub/efi/api.h | 14 ++++--- 29 4 files changed, 79 insertions(+), 21 deletions(-) 30 31diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c 32index dc31caa21..c97969a65 100644 33--- a/grub-core/kern/efi/efi.c 34+++ b/grub-core/kern/efi/efi.c 35@@ -332,7 +332,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) 36 37 dp = dp0; 38 39- while (1) 40+ while (dp) 41 { 42 grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp); 43 grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp); 44@@ -342,9 +342,15 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) 45 if (type == GRUB_EFI_MEDIA_DEVICE_PATH_TYPE 46 && subtype == GRUB_EFI_FILE_PATH_DEVICE_PATH_SUBTYPE) 47 { 48- grub_efi_uint16_t len; 49- len = ((GRUB_EFI_DEVICE_PATH_LENGTH (dp) - 4) 50- / sizeof (grub_efi_char16_t)); 51+ grub_efi_uint16_t len = GRUB_EFI_DEVICE_PATH_LENGTH (dp); 52+ 53+ if (len < 4) 54+ { 55+ grub_error (GRUB_ERR_OUT_OF_RANGE, 56+ "malformed EFI Device Path node has length=%d", len); 57+ return NULL; 58+ } 59+ len = (len - 4) / sizeof (grub_efi_char16_t); 60 filesize += GRUB_MAX_UTF8_PER_UTF16 * len + 2; 61 } 62 63@@ -360,7 +366,7 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) 64 if (!name) 65 return NULL; 66 67- while (1) 68+ while (dp) 69 { 70 grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp); 71 grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp); 72@@ -376,8 +382,15 @@ grub_efi_get_filename (grub_efi_device_path_t *dp0) 73 74 *p++ = '/'; 75 76- len = ((GRUB_EFI_DEVICE_PATH_LENGTH (dp) - 4) 77- / sizeof (grub_efi_char16_t)); 78+ len = GRUB_EFI_DEVICE_PATH_LENGTH (dp); 79+ if (len < 4) 80+ { 81+ grub_error (GRUB_ERR_OUT_OF_RANGE, 82+ "malformed EFI Device Path node has length=%d", len); 83+ return NULL; 84+ } 85+ 86+ len = (len - 4) / sizeof (grub_efi_char16_t); 87 fp = (grub_efi_file_path_device_path_t *) dp; 88 /* According to EFI spec Path Name is NULL terminated */ 89 while (len > 0 && fp->path_name[len - 1] == 0) 90@@ -452,7 +465,26 @@ grub_efi_duplicate_device_path (const grub_efi_device_path_t *dp) 91 ; 92 p = GRUB_EFI_NEXT_DEVICE_PATH (p)) 93 { 94- total_size += GRUB_EFI_DEVICE_PATH_LENGTH (p); 95+ grub_size_t len = GRUB_EFI_DEVICE_PATH_LENGTH (p); 96+ 97+ /* 98+ * In the event that we find a node that's completely garbage, for 99+ * example if we get to 0x7f 0x01 0x02 0x00 ... (EndInstance with a size 100+ * of 2), GRUB_EFI_END_ENTIRE_DEVICE_PATH() will be true and 101+ * GRUB_EFI_NEXT_DEVICE_PATH() will return NULL, so we won't continue, 102+ * and neither should our consumers, but there won't be any error raised 103+ * even though the device path is junk. 104+ * 105+ * This keeps us from passing junk down back to our caller. 106+ */ 107+ if (len < 4) 108+ { 109+ grub_error (GRUB_ERR_OUT_OF_RANGE, 110+ "malformed EFI Device Path node has length=%d", len); 111+ return NULL; 112+ } 113+ 114+ total_size += len; 115 if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (p)) 116 break; 117 } 118@@ -497,7 +529,7 @@ dump_vendor_path (const char *type, grub_efi_vendor_device_path_t *vendor) 119 void 120 grub_efi_print_device_path (grub_efi_device_path_t *dp) 121 { 122- while (1) 123+ while (GRUB_EFI_DEVICE_PATH_VALID (dp)) 124 { 125 grub_efi_uint8_t type = GRUB_EFI_DEVICE_PATH_TYPE (dp); 126 grub_efi_uint8_t subtype = GRUB_EFI_DEVICE_PATH_SUBTYPE (dp); 127@@ -909,7 +941,10 @@ grub_efi_compare_device_paths (const grub_efi_device_path_t *dp1, 128 /* Return non-zero. */ 129 return 1; 130 131- while (1) 132+ if (dp1 == dp2) 133+ return 0; 134+ 135+ while (GRUB_EFI_DEVICE_PATH_VALID (dp1) && GRUB_EFI_DEVICE_PATH_VALID (dp2)) 136 { 137 grub_efi_uint8_t type1, type2; 138 grub_efi_uint8_t subtype1, subtype2; 139@@ -945,5 +980,14 @@ grub_efi_compare_device_paths (const grub_efi_device_path_t *dp1, 140 dp2 = (grub_efi_device_path_t *) ((char *) dp2 + len2); 141 } 142 143+ /* 144+ * There's no "right" answer here, but we probably don't want to call a valid 145+ * dp and an invalid dp equal, so pick one way or the other. 146+ */ 147+ if (GRUB_EFI_DEVICE_PATH_VALID (dp1) && !GRUB_EFI_DEVICE_PATH_VALID (dp2)) 148+ return 1; 149+ else if (!GRUB_EFI_DEVICE_PATH_VALID (dp1) && GRUB_EFI_DEVICE_PATH_VALID (dp2)) 150+ return -1; 151+ 152 return 0; 153 } 154diff --git a/grub-core/loader/efi/chainloader.c b/grub-core/loader/efi/chainloader.c 155index daf8c6b54..a8d7b9155 100644 156--- a/grub-core/loader/efi/chainloader.c 157+++ b/grub-core/loader/efi/chainloader.c 158@@ -156,9 +156,18 @@ make_file_path (grub_efi_device_path_t *dp, const char *filename) 159 160 size = 0; 161 d = dp; 162- while (1) 163+ while (d) 164 { 165- size += GRUB_EFI_DEVICE_PATH_LENGTH (d); 166+ grub_size_t len = GRUB_EFI_DEVICE_PATH_LENGTH (d); 167+ 168+ if (len < 4) 169+ { 170+ grub_error (GRUB_ERR_OUT_OF_RANGE, 171+ "malformed EFI Device Path node has length=%d", len); 172+ return NULL; 173+ } 174+ 175+ size += len; 176 if ((GRUB_EFI_END_ENTIRE_DEVICE_PATH (d))) 177 break; 178 d = GRUB_EFI_NEXT_DEVICE_PATH (d); 179diff --git a/grub-core/loader/i386/xnu.c b/grub-core/loader/i386/xnu.c 180index e9e119259..a70093607 100644 181--- a/grub-core/loader/i386/xnu.c 182+++ b/grub-core/loader/i386/xnu.c 183@@ -515,14 +515,15 @@ grub_cmd_devprop_load (grub_command_t cmd __attribute__ ((unused)), 184 185 devhead = buf; 186 buf = devhead + 1; 187- dpstart = buf; 188+ dp = dpstart = buf; 189 190- do 191+ while (GRUB_EFI_DEVICE_PATH_VALID (dp) && buf < bufend) 192 { 193- dp = buf; 194 buf = (char *) buf + GRUB_EFI_DEVICE_PATH_LENGTH (dp); 195+ if (GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp)) 196+ break; 197+ dp = buf; 198 } 199- while (!GRUB_EFI_END_ENTIRE_DEVICE_PATH (dp) && buf < bufend); 200 201 dev = grub_xnu_devprop_add_device (dpstart, (char *) buf 202 - (char *) dpstart); 203diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h 204index addcbfa8f..cf1355a8c 100644 205--- a/include/grub/efi/api.h 206+++ b/include/grub/efi/api.h 207@@ -625,6 +625,7 @@ typedef struct grub_efi_device_path grub_efi_device_path_protocol_t; 208 #define GRUB_EFI_DEVICE_PATH_TYPE(dp) ((dp)->type & 0x7f) 209 #define GRUB_EFI_DEVICE_PATH_SUBTYPE(dp) ((dp)->subtype) 210 #define GRUB_EFI_DEVICE_PATH_LENGTH(dp) ((dp)->length) 211+#define GRUB_EFI_DEVICE_PATH_VALID(dp) ((dp) != NULL && GRUB_EFI_DEVICE_PATH_LENGTH (dp) >= 4) 212 213 /* The End of Device Path nodes. */ 214 #define GRUB_EFI_END_DEVICE_PATH_TYPE (0xff & 0x7f) 215@@ -633,13 +634,16 @@ typedef struct grub_efi_device_path grub_efi_device_path_protocol_t; 216 #define GRUB_EFI_END_THIS_DEVICE_PATH_SUBTYPE 0x01 217 218 #define GRUB_EFI_END_ENTIRE_DEVICE_PATH(dp) \ 219- (GRUB_EFI_DEVICE_PATH_TYPE (dp) == GRUB_EFI_END_DEVICE_PATH_TYPE \ 220- && (GRUB_EFI_DEVICE_PATH_SUBTYPE (dp) \ 221- == GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE)) 222+ (!GRUB_EFI_DEVICE_PATH_VALID (dp) || \ 223+ (GRUB_EFI_DEVICE_PATH_TYPE (dp) == GRUB_EFI_END_DEVICE_PATH_TYPE \ 224+ && (GRUB_EFI_DEVICE_PATH_SUBTYPE (dp) \ 225+ == GRUB_EFI_END_ENTIRE_DEVICE_PATH_SUBTYPE))) 226 227 #define GRUB_EFI_NEXT_DEVICE_PATH(dp) \ 228- ((grub_efi_device_path_t *) ((char *) (dp) \ 229- + GRUB_EFI_DEVICE_PATH_LENGTH (dp))) 230+ (GRUB_EFI_DEVICE_PATH_VALID (dp) \ 231+ ? ((grub_efi_device_path_t *) \ 232+ ((char *) (dp) + GRUB_EFI_DEVICE_PATH_LENGTH (dp))) \ 233+ : NULL) 234 235 /* Hardware Device Path. */ 236 #define GRUB_EFI_HARDWARE_DEVICE_PATH_TYPE 1 237-- 2382.26.2 239 240