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