From 43651027d24e62a7a463254165e1e46e42aecdea Mon Sep 17 00:00:00 2001 From: Maxim Suhanov Date: Mon, 28 Aug 2023 16:31:57 +0300 Subject: fs/ntfs: Fix an OOB write when parsing the $ATTRIBUTE_LIST attribute for the $MFT file When parsing an extremely fragmented $MFT file, i.e., the file described using the $ATTRIBUTE_LIST attribute, current NTFS code will reuse a buffer containing bytes read from the underlying drive to store sector numbers, which are consumed later to read data from these sectors into another buffer. These sectors numbers, two 32-bit integers, are always stored at predefined offsets, 0x10 and 0x14, relative to first byte of the selected entry within the $ATTRIBUTE_LIST attribute. Usually, this won't cause any problem. However, when parsing a specially-crafted file system image, this may cause the NTFS code to write these integers beyond the buffer boundary, likely causing the GRUB memory allocator to misbehave or fail. These integers contain values which are controlled by on-disk structures of the NTFS file system. Such modification and resulting misbehavior may touch a memory range not assigned to the GRUB and owned by firmware or another EFI application/driver. This fix introduces checks to ensure that these sector numbers are never written beyond the boundary. Fixes: CVE-2023-4692 Reported-by: Maxim Suhanov Signed-off-by: Maxim Suhanov Reviewed-by: Daniel Kiper --- grub-core/fs/ntfs.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c index bbdbe24..c3c4db1 100644 --- a/grub-core/fs/ntfs.c +++ b/grub-core/fs/ntfs.c @@ -184,7 +184,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr) } if (at->attr_end) { - grub_uint8_t *pa; + grub_uint8_t *pa, *pa_end; at->emft_buf = grub_malloc (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR); if (at->emft_buf == NULL) @@ -209,11 +209,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr) } at->attr_nxt = at->edat_buf; at->attr_end = at->edat_buf + u32at (pa, 0x30); + pa_end = at->edat_buf + n; } else { at->attr_nxt = at->attr_end + u16at (pa, 0x14); at->attr_end = at->attr_end + u32at (pa, 4); + pa_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR); } at->flags |= GRUB_NTFS_AF_ALST; while (at->attr_nxt < at->attr_end) @@ -230,6 +232,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr) at->flags |= GRUB_NTFS_AF_GPOS; at->attr_cur = at->attr_nxt; pa = at->attr_cur; + + if ((pa >= pa_end) || (pa_end - pa < 0x18)) + { + grub_error (GRUB_ERR_BAD_FS, "can\'t parse attribute list"); + return NULL; + } + grub_set_unaligned32 ((char *) pa + 0x10, grub_cpu_to_le32 (at->mft->data->mft_start)); grub_set_unaligned32 ((char *) pa + 0x14, @@ -240,6 +249,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr) { if (*pa != attr) break; + + if ((pa >= pa_end) || (pa_end - pa < 0x18)) + { + grub_error (GRUB_ERR_BAD_FS, "can\'t parse attribute list"); + return NULL; + } + if (read_attr (at, pa + 0x10, u32at (pa, 0x10) * (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR), -- cgit v1.1 From 0ed2458cc4eff6d9a9199527e2a0b6d445802f94 Mon Sep 17 00:00:00 2001 From: Maxim Suhanov Date: Mon, 28 Aug 2023 16:32:33 +0300 Subject: fs/ntfs: Fix an OOB read when reading data from the resident $DATA attribute When reading a file containing resident data, i.e., the file data is stored in the $DATA attribute within the NTFS file record, not in external clusters, there are no checks that this resident data actually fits the corresponding file record segment. When parsing a specially-crafted file system image, the current NTFS code will read the file data from an arbitrary, attacker-chosen memory offset and of arbitrary, attacker-chosen length. This allows an attacker to display arbitrary chunks of memory, which could contain sensitive information like password hashes or even plain-text, obfuscated passwords from BS EFI variables. This fix implements a check to ensure that resident data is read from the corresponding file record segment only. Fixes: CVE-2023-4693 Reported-by: Maxim Suhanov Signed-off-by: Maxim Suhanov Reviewed-by: Daniel Kiper --- grub-core/fs/ntfs.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c index c3c4db1..a68e173 100644 --- a/grub-core/fs/ntfs.c +++ b/grub-core/fs/ntfs.c @@ -401,7 +401,18 @@ read_data (struct grub_ntfs_attr *at, grub_uint8_t *pa, grub_uint8_t *dest, { if (ofs + len > u32at (pa, 0x10)) return grub_error (GRUB_ERR_BAD_FS, "read out of range"); - grub_memcpy (dest, pa + u32at (pa, 0x14) + ofs, len); + + if (u32at (pa, 0x10) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR)) + return grub_error (GRUB_ERR_BAD_FS, "resident attribute too large"); + + if (pa >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR)) + return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range"); + + if (u16at (pa, 0x14) + u32at (pa, 0x10) > + (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) pa) + return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range"); + + grub_memcpy (dest, pa + u16at (pa, 0x14) + ofs, len); return 0; } -- cgit v1.1 From 7e5f031a6a6a3decc2360a7b0c71abbe598e7354 Mon Sep 17 00:00:00 2001 From: Maxim Suhanov Date: Mon, 28 Aug 2023 16:33:17 +0300 Subject: fs/ntfs: Fix an OOB read when parsing directory entries from resident and non-resident index attributes This fix introduces checks to ensure that index entries are never read beyond the corresponding directory index. The lack of this check is a minor issue, likely not exploitable in any way. Reported-by: Maxim Suhanov Signed-off-by: Maxim Suhanov Reviewed-by: Daniel Kiper --- grub-core/fs/ntfs.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c index a68e173..2d78b96 100644 --- a/grub-core/fs/ntfs.c +++ b/grub-core/fs/ntfs.c @@ -599,7 +599,7 @@ get_utf8 (grub_uint8_t *in, grub_size_t len) } static int -list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos, +list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos, grub_uint8_t *end_pos, grub_fshelp_iterate_dir_hook_t hook, void *hook_data) { grub_uint8_t *np; @@ -610,6 +610,9 @@ list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos, grub_uint8_t namespace; char *ustr; + if ((pos >= end_pos) || (end_pos - pos < 0x52)) + break; + if (pos[0xC] & 2) /* end signature */ break; @@ -617,6 +620,9 @@ list_file (struct grub_ntfs_file *diro, grub_uint8_t *pos, ns = *(np++); namespace = *(np++); + if (2 * ns > end_pos - pos - 0x52) + break; + /* * Ignore files in DOS namespace, as they will reappear as Win32 * names. @@ -806,7 +812,9 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir, } cur_pos += 0x10; /* Skip index root */ - ret = list_file (mft, cur_pos + u16at (cur_pos, 0), hook, hook_data); + ret = list_file (mft, cur_pos + u16at (cur_pos, 0), + at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR), + hook, hook_data); if (ret) goto done; @@ -893,6 +901,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir, (const grub_uint8_t *) "INDX"))) goto done; ret = list_file (mft, &indx[0x18 + u16at (indx, 0x18)], + indx + (mft->data->idx_size << GRUB_NTFS_BLK_SHR), hook, hook_data); if (ret) goto done; -- cgit v1.1 From 7a5a116739fa6d8a625da7d6b9272c9a2462f967 Mon Sep 17 00:00:00 2001 From: Maxim Suhanov Date: Mon, 28 Aug 2023 16:33:44 +0300 Subject: fs/ntfs: Fix an OOB read when parsing bitmaps for index attributes This fix introduces checks to ensure that bitmaps for directory indices are never read beyond their actual sizes. The lack of this check is a minor issue, likely not exploitable in any way. Reported-by: Maxim Suhanov Signed-off-by: Maxim Suhanov Reviewed-by: Daniel Kiper --- grub-core/fs/ntfs.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c index 2d78b96..bb70c89 100644 --- a/grub-core/fs/ntfs.c +++ b/grub-core/fs/ntfs.c @@ -843,6 +843,25 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir, if (is_resident) { + if (bitmap_len > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR)) + { + grub_error (GRUB_ERR_BAD_FS, "resident bitmap too large"); + goto done; + } + + if (cur_pos >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR)) + { + grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range"); + goto done; + } + + if (u16at (cur_pos, 0x14) + u32at (cur_pos, 0x10) > + (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) cur_pos) + { + grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range"); + goto done; + } + grub_memcpy (bmp, cur_pos + u16at (cur_pos, 0x14), bitmap_len); } -- cgit v1.1 From 1fe82c41e070385e273d7bb1cfb482627a3c28e8 Mon Sep 17 00:00:00 2001 From: Maxim Suhanov Date: Mon, 28 Aug 2023 16:38:19 +0300 Subject: fs/ntfs: Fix an OOB read when parsing a volume label This fix introduces checks to ensure that an NTFS volume label is always read from the corresponding file record segment. The current NTFS code allows the volume label string to be read from an arbitrary, attacker-chosen memory location. However, the bytes read are always treated as UTF-16LE. So, the final string displayed is mostly unreadable and it can't be easily converted back to raw bytes. The lack of this check is a minor issue, likely not causing a significant data leak. Reported-by: Maxim Suhanov Signed-off-by: Maxim Suhanov Reviewed-by: Daniel Kiper --- grub-core/fs/ntfs.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c index bb70c89..ff5e374 100644 --- a/grub-core/fs/ntfs.c +++ b/grub-core/fs/ntfs.c @@ -1213,13 +1213,29 @@ grub_ntfs_label (grub_device_t device, char **label) init_attr (&mft->attr, mft); pa = find_attr (&mft->attr, GRUB_NTFS_AT_VOLUME_NAME); + + if (pa >= mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR)) + { + grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label"); + goto fail; + } + + if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa < 0x16) + { + grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label"); + goto fail; + } + if ((pa) && (pa[8] == 0) && (u32at (pa, 0x10))) { int len; len = u32at (pa, 0x10) / 2; pa += u16at (pa, 0x14); - *label = get_utf8 (pa, len); + if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa >= 2 * len) + *label = get_utf8 (pa, len); + else + grub_error (GRUB_ERR_BAD_FS, "can\'t parse volume label"); } fail: -- cgit v1.1 From e58b870ff926415e23fc386af41ff81b2f588763 Mon Sep 17 00:00:00 2001 From: Maxim Suhanov Date: Mon, 28 Aug 2023 16:40:07 +0300 Subject: fs/ntfs: Make code more readable Move some calls used to access NTFS attribute header fields into functions with human-readable names. Suggested-by: Daniel Kiper Signed-off-by: Maxim Suhanov Reviewed-by: Daniel Kiper --- grub-core/fs/ntfs.c | 48 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c index ff5e374..de435aa 100644 --- a/grub-core/fs/ntfs.c +++ b/grub-core/fs/ntfs.c @@ -52,6 +52,24 @@ u64at (void *ptr, grub_size_t ofs) return grub_le_to_cpu64 (grub_get_unaligned64 ((char *) ptr + ofs)); } +static grub_uint16_t +first_attr_off (void *mft_buf_ptr) +{ + return u16at (mft_buf_ptr, 0x14); +} + +static grub_uint16_t +res_attr_data_off (void *res_attr_ptr) +{ + return u16at (res_attr_ptr, 0x14); +} + +static grub_uint32_t +res_attr_data_len (void *res_attr_ptr) +{ + return u32at (res_attr_ptr, 0x10); +} + grub_ntfscomp_func_t grub_ntfscomp_func; static grub_err_t @@ -106,7 +124,7 @@ init_attr (struct grub_ntfs_attr *at, struct grub_ntfs_file *mft) { at->mft = mft; at->flags = (mft == &mft->data->mmft) ? GRUB_NTFS_AF_MMFT : 0; - at->attr_nxt = mft->buf + u16at (mft->buf, 0x14); + at->attr_nxt = mft->buf + first_attr_off (mft->buf); at->attr_end = at->emft_buf = at->edat_buf = at->sbuf = NULL; } @@ -154,7 +172,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr) return NULL; } - new_pos = &at->emft_buf[u16at (at->emft_buf, 0x14)]; + new_pos = &at->emft_buf[first_attr_off (at->emft_buf)]; while (*new_pos != 0xFF) { if ((*new_pos == *at->attr_cur) @@ -213,7 +231,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr) } else { - at->attr_nxt = at->attr_end + u16at (pa, 0x14); + at->attr_nxt = at->attr_end + res_attr_data_off (pa); at->attr_end = at->attr_end + u32at (pa, 4); pa_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR); } @@ -399,20 +417,20 @@ read_data (struct grub_ntfs_attr *at, grub_uint8_t *pa, grub_uint8_t *dest, if (pa[8] == 0) { - if (ofs + len > u32at (pa, 0x10)) + if (ofs + len > res_attr_data_len (pa)) return grub_error (GRUB_ERR_BAD_FS, "read out of range"); - if (u32at (pa, 0x10) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR)) + if (res_attr_data_len (pa) > (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR)) return grub_error (GRUB_ERR_BAD_FS, "resident attribute too large"); if (pa >= at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR)) return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range"); - if (u16at (pa, 0x14) + u32at (pa, 0x10) > + if (res_attr_data_off (pa) + res_attr_data_len (pa) > (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) pa) return grub_error (GRUB_ERR_BAD_FS, "resident attribute out of range"); - grub_memcpy (dest, pa + u16at (pa, 0x14) + ofs, len); + grub_memcpy (dest, pa + res_attr_data_off (pa) + ofs, len); return 0; } @@ -556,7 +574,7 @@ init_file (struct grub_ntfs_file *mft, grub_uint64_t mftno) (unsigned long long) mftno); if (!pa[8]) - mft->size = u32at (pa, 0x10); + mft->size = res_attr_data_len (pa); else mft->size = u64at (pa, 0x30); @@ -805,7 +823,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir, (u32at (cur_pos, 0x18) != 0x490024) || (u32at (cur_pos, 0x1C) != 0x300033)) continue; - cur_pos += u16at (cur_pos, 0x14); + cur_pos += res_attr_data_off (cur_pos); if (*cur_pos != 0x30) /* Not filename index */ continue; break; @@ -834,7 +852,7 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir, { int is_resident = (cur_pos[8] == 0); - bitmap_len = ((is_resident) ? u32at (cur_pos, 0x10) : + bitmap_len = ((is_resident) ? res_attr_data_len (cur_pos) : u32at (cur_pos, 0x28)); bmp = grub_malloc (bitmap_len); @@ -855,14 +873,14 @@ grub_ntfs_iterate_dir (grub_fshelp_node_t dir, goto done; } - if (u16at (cur_pos, 0x14) + u32at (cur_pos, 0x10) > + if (res_attr_data_off (cur_pos) + res_attr_data_len (cur_pos) > (grub_addr_t) at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR) - (grub_addr_t) cur_pos) { grub_error (GRUB_ERR_BAD_FS, "resident bitmap out of range"); goto done; } - grub_memcpy (bmp, cur_pos + u16at (cur_pos, 0x14), + grub_memcpy (bmp, cur_pos + res_attr_data_off (cur_pos), bitmap_len); } else @@ -1226,12 +1244,12 @@ grub_ntfs_label (grub_device_t device, char **label) goto fail; } - if ((pa) && (pa[8] == 0) && (u32at (pa, 0x10))) + if ((pa) && (pa[8] == 0) && (res_attr_data_len (pa))) { int len; - len = u32at (pa, 0x10) / 2; - pa += u16at (pa, 0x14); + len = res_attr_data_len (pa) / 2; + pa += res_attr_data_off (pa); if (mft->buf + (mft->data->mft_size << GRUB_NTFS_BLK_SHR) - pa >= 2 * len) *label = get_utf8 (pa, len); else -- cgit v1.1