Correct ntfs_test test failures around attempting to validate attribute
list entries as attributes. The NTFS code uses common logic in some
places to parse both attributes and attribute_lists which complicates
validation. Attribute lists contain different headers including a
different size of the length field (2 bytes) at offset 4 instead of the
4 byte length field used in attributes at offset 4. There are other
differences as well, but attempting to validate attribute list types
using attribute header validation was causing failure of the NTFS test
suite. This change restores some of the validation logic which may be
shared between attributes and attribute lists to be closer to the
original logic prior to fixes for previous CVEs. A following commit will
address some of the implications of removing this validation logic by
correcting some fuzzer failures (some which are exposed by removing the
validation in some of the cases).
Fixes: 067b6d225 (fs/ntfs: Implement attribute verification)
Signed-off-by: Andrew Hamilton <adham...@gmail.com>
---
grub-core/fs/ntfs.c | 45 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 3eb70111b..0d087acd8 100644
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -219,7 +219,7 @@ validate_attribute (grub_uint8_t *attr, void *end)
/* Return the next attribute if it exists, otherwise return NULL. */
static grub_uint8_t *
-next_attribute (grub_uint8_t *curr_attribute, void *end)
+next_attribute (grub_uint8_t *curr_attribute, void *end, bool validate)
{
grub_uint8_t *next = curr_attribute;
@@ -231,7 +231,7 @@ next_attribute (grub_uint8_t *curr_attribute, void *end)
return NULL;
next += u16at (curr_attribute, 4);
- if (validate_attribute (next, end) == false)
+ if (validate && validate_attribute (next, end) == false)
return NULL;
return next;
@@ -316,13 +316,16 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
{
grub_uint8_t *mft_end;
+ /* GRUB_NTFS_AF_ALST indicates the attribute list type */
if (at->flags & GRUB_NTFS_AF_ALST)
{
retry:
while (at->attr_nxt)
{
at->attr_cur = at->attr_nxt;
- at->attr_nxt = next_attribute (at->attr_cur, at->attr_end);
+ /* Go to the next attribute in the list but do not validate */
+ /* because this is the attribute list type. */
+ at->attr_nxt = next_attribute (at->attr_cur, at->attr_end, false);
if ((*at->attr_cur == attr) || (attr == 0))
{
grub_uint8_t *new_pos, *end;
@@ -378,7 +381,9 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
{
return new_pos;
}
- new_pos = next_attribute (new_pos, end);
+ /* Go to the next attribute in the list but do not validate
*/
+ /* because this is the attribute list type. */
+ new_pos = next_attribute (new_pos, end, false);
}
grub_error (GRUB_ERR_BAD_FS,
"can\'t find 0x%X in attribute list",
@@ -392,7 +397,19 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
mft_end = at->mft->buf + (at->mft->data->mft_size << GRUB_NTFS_BLK_SHR);
while (at->attr_cur >= at->mft->buf && at->attr_cur < mft_end &&
*at->attr_cur != 0xFF)
{
- at->attr_nxt = next_attribute (at->attr_cur, at->end);
+ /* We can't use validate_attribute here because this logic
+ * seems to be used for both parsing through attributes
+ * and attribute lists. */
+ grub_uint16_t nsize = u16at (at->attr_cur, 4);
+ if (at->attr_cur + nsize >= at->end ||
+ at->attr_cur + GRUB_NTFS_ATTRIBUTE_HEADER_SIZE >= at->end)
+ {
+ at->attr_nxt = at->attr_cur;
+ break;
+ }
+ else
+ at->attr_nxt = at->attr_cur + nsize;
+
if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST)
at->attr_end = at->attr_cur;
if ((*at->attr_cur == attr) || (attr == 0))
@@ -439,14 +456,18 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
/* From this point on pa_end is the end of the buffer */
at->end = pa_end;
- if (validate_attribute (at->attr_nxt, pa_end) == false)
- return NULL;
+ if (at->attr_end >= pa_end || at->attr_nxt >= pa_end)
+ return NULL;
while (at->attr_nxt)
{
if ((*at->attr_nxt == attr) || (attr == 0))
break;
- at->attr_nxt = next_attribute (at->attr_nxt, pa_end);
+
+ grub_uint16_t nxt_offset = u16at (at->attr_nxt, 4);
+ at->attr_nxt += nxt_offset;
+ if (nxt_offset == 0 || at->attr_nxt >= (pa_end - 4))
+ at->attr_nxt = NULL;
}
if (at->attr_nxt >= at->attr_end || at->attr_nxt == NULL)
@@ -471,7 +492,7 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
+ 1));
pa = at->attr_nxt + u16at (pa, 4);
- if (validate_attribute (pa, pa_end) == true)
+ if (pa >= pa_end)
pa = NULL;
while (pa)
@@ -490,7 +511,9 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
u32at (pa, 0x10) * (at->mft->data->mft_size <<
GRUB_NTFS_BLK_SHR),
at->mft->data->mft_size << GRUB_NTFS_BLK_SHR, 0, 0, 0))
return NULL;
- pa = next_attribute (pa, pa_end);
+ pa += u16at(pa, 4);
+ if (pa >= pa_end)
+ pa = NULL;
}
at->attr_nxt = at->attr_cur;
at->flags &= ~GRUB_NTFS_AF_GPOS;
@@ -737,7 +760,7 @@ read_attr (struct grub_ntfs_attr *at, grub_uint8_t *dest,
grub_disk_addr_t ofs,
if (u32at (pa, 8) > vcn)
break;
at->attr_nxt = pa;
- pa = next_attribute (pa, at->attr_end);
+ pa = next_attribute (pa, at->attr_end, true);
}
}
pp = find_attr (at, attr);
--
2.39.5
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
