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>

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);