fs/ntfs: Use a helper function to access attributes

Right now to access the next attribute the code reads the length of the
current attribute and adds that to the current pointer. This is error
prone as bounds checking needs to be performed all over the place. So,
implement a helper and ensure its used across find_attr() and read_attr().

This commit does *not* implement full bounds checking. It is just the
preparation work for this to be added into the helper.

Signed-off-by: B Horn <b@horn.uk>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index dbda720e1ee9b8701ef45e735c57991846dd0c15..1c678f3d0e06c68801a1d9ec8ea75994dad1418e 100644 (file)
--- a/grub-core/fs/ntfs.c
+++ b/grub-core/fs/ntfs.c
@@ -70,6 +70,25 @@ res_attr_data_len (void *res_attr_ptr)
   return u32at (res_attr_ptr, 0x10);
 }
 
+/* Return the next attribute if it exists, otherwise return NULL. */
+static grub_uint8_t *
+next_attribute (grub_uint8_t *curr_attribute, void *end)
+{
+  grub_uint8_t *next = curr_attribute;
+
+  /*
+   * Need to verify we aren't exceeding the end of the buffer by reading the
+   * header for the current attribute
+   */
+  if (curr_attribute + GRUB_NTFS_ATTRIBUTE_HEADER_SIZE >= (grub_uint8_t *) end)
+    return NULL;
+
+  next += u16at (curr_attribute, 4);
+
+  return next;
+}
+
+
 grub_ntfscomp_func_t grub_ntfscomp_func;
 
 static grub_err_t
@@ -151,13 +170,13 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
   if (at->flags & GRUB_NTFS_AF_ALST)
     {
     retry:
-      while (at->attr_nxt < at->attr_end)
+      while (at->attr_nxt)
        {
          at->attr_cur = at->attr_nxt;
-         at->attr_nxt += u16at (at->attr_cur, 4);
+         at->attr_nxt = next_attribute (at->attr_cur, at->attr_end);
          if ((*at->attr_cur == attr) || (attr == 0))
            {
-             grub_uint8_t *new_pos;
+             grub_uint8_t *new_pos, *end;
 
              if (at->flags & GRUB_NTFS_AF_MMFT)
                {
@@ -181,15 +200,36 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
                    return NULL;
                }
 
+             /*
+              * Only time emft_bufs is defined is in this function, with this
+              * size.
+              */
+             grub_size_t emft_buf_size =
+               at->mft->data->mft_size << GRUB_NTFS_BLK_SHR;
+
+             /*
+              * Needs to be enough space for the successful case to even
+              * bother.
+              */
+             if (first_attr_off (at->emft_buf) >= (emft_buf_size - 0x18 - 2))
+               {
+                 grub_error (GRUB_ERR_BAD_FS,
+                             "can\'t find 0x%X in attribute list",
+                             (unsigned char) *at->attr_cur);
+                 return NULL;
+               }
+
              new_pos = &at->emft_buf[first_attr_off (at->emft_buf)];
-             while (*new_pos != 0xFF)
+             end = &at->emft_buf[emft_buf_size];
+
+             while (new_pos && *new_pos != 0xFF)
                {
                  if ((*new_pos == *at->attr_cur)
                      && (u16at (new_pos, 0xE) == u16at (at->attr_cur, 0x18)))
                    {
                      return new_pos;
                    }
-                 new_pos += u16at (new_pos, 4);
+                 new_pos = next_attribute (new_pos, end);
                }
              grub_error (GRUB_ERR_BAD_FS,
                          "can\'t find 0x%X in attribute list",
@@ -203,7 +243,7 @@ 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 < mft_end && *at->attr_cur != 0xFF)
     {
-      at->attr_nxt += u16at (at->attr_cur, 4);
+      at->attr_nxt = next_attribute (at->attr_cur, at->end);
       if (*at->attr_cur == GRUB_NTFS_AT_ATTRIBUTE_LIST)
        at->attr_end = at->attr_cur;
       if ((*at->attr_cur == attr) || (attr == 0))
@@ -250,13 +290,14 @@ 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;
 
-      while (at->attr_nxt < at->attr_end)
+      while (at->attr_nxt)
        {
          if ((*at->attr_nxt == attr) || (attr == 0))
            break;
-         at->attr_nxt += u16at (at->attr_nxt, 4);
+         at->attr_nxt = next_attribute (at->attr_nxt, pa_end);
        }
-      if (at->attr_nxt >= at->attr_end)
+
+      if (at->attr_nxt >= at->attr_end || at->attr_nxt == NULL)
        return NULL;
 
       if ((at->flags & GRUB_NTFS_AF_MMFT) && (attr == GRUB_NTFS_AT_DATA))
@@ -277,7 +318,8 @@ find_attr (struct grub_ntfs_attr *at, grub_uint8_t attr)
                                grub_cpu_to_le32 (at->mft->data->mft_start
                                                  + 1));
          pa = at->attr_nxt + u16at (pa, 4);
-         while (pa < at->attr_end)
+
+         while (pa)
            {
              if (*pa != attr)
                break;
@@ -293,7 +335,7 @@ 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 += u16at (pa, 4);
+             pa = next_attribute (pa, pa_end);
            }
          at->attr_nxt = at->attr_cur;
          at->flags &= ~GRUB_NTFS_AF_GPOS;
@@ -530,14 +572,15 @@ read_attr (struct grub_ntfs_attr *at, grub_uint8_t *dest, grub_disk_addr_t ofs,
       else
        vcn = ofs >> (at->mft->data->log_spc + GRUB_NTFS_BLK_SHR);
       pa = at->attr_nxt + u16at (at->attr_nxt, 4);
-      while (pa < at->attr_end)
+
+      while (pa)
        {
          if (*pa != attr)
            break;
          if (u32at (pa, 8) > vcn)
            break;
          at->attr_nxt = pa;
-         pa += u16at (pa, 4);
+         pa = next_attribute (pa, at->attr_end);
        }
     }
   pp = find_attr (at, attr);

diff --git a/include/grub/ntfs.h b/include/grub/ntfs.h
index ec1c4db3833cf622c03c8ff100b9dacaee1230fa..2c80784039f3f9c79617b78e933807ca89ec28c6 100644 (file)
--- a/include/grub/ntfs.h
+++ b/include/grub/ntfs.h
@@ -89,6 +89,8 @@ enum
 #define GRUB_NTFS_COM_SEC              (GRUB_NTFS_COM_LEN >> GRUB_NTFS_BLK_SHR)
 #define GRUB_NTFS_LOG_COM_SEC          (GRUB_NTFS_COM_LOG_LEN - GRUB_NTFS_BLK_SHR)
 
+#define GRUB_NTFS_ATTRIBUTE_HEADER_SIZE 16
+
 enum
   {
     GRUB_NTFS_AF_ALST          = 1,