grub2/0371-fs-ntfs-Use-a-helper-function-to-access-attributes.patch

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: B Horn <b@horn.uk>
Date: Tue, 14 May 2024 12:39:56 +0100
Subject: [PATCH] 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 | 69 +++++++++++++++++++++++++++++++++++++++++++----------
 include/grub/ntfs.h |  2 ++
 2 files changed, 58 insertions(+), 13 deletions(-)

diff --git a/grub-core/fs/ntfs.c b/grub-core/fs/ntfs.c
index 562b4f7eb..9311ba9ef 100644
--- 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 ec1c4db38..2c8078403 100644
--- 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,