[PATCH 2/2] ntfs: validate resident file name attribute length

[DaeMyung Kang]

ntfs_attr_find() and ntfs_external_attr_find() check that generic
resident attribute values fit in their attribute records and that
fixed-size resident values are large enough. For $FILE_NAME, however,
the fixed part is not enough: the value also contains a variable-length
UTF-16 name whose length is stored in file_name_length.

A crafted image can set a small resident value_length while leaving
file_name_length large. Callers then trust file_name_length and read
past the resident value when converting or comparing the name. This was
reproduced with a crafted image under KASAN as a slab-out-of-bounds read
from the kmalloc-1k MFT record copy, for example through:

  ntfs_lookup()
  ntfs_iget()
  ntfs_read_locked_inode()
  ntfs_attr_name_get()
  ntfs_ucstonls()
  utf16s_to_utf8s()

Validate $FILE_NAME before lookup can return the current attribute,
including the AT_UNUSED enumeration case where callers inspect returned
attributes directly. Reuse the same self-contained helper for both the
base-record lookup path and the attribute-list lookup path. Log a
specific corruption message at the rejection point, matching the
existing attribute-name validation in ntfs_attr_find().

Reject non-resident $FILE_NAME records too: the format requires
$FILE_NAME to be resident and callers treat returned records as
resident.

Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
Signed-off-by: DaeMyung Kang <charsyam@xxxxxxxxx>
---
 fs/ntfs/attrib.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..5643a097eb11 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -595,6 +595,37 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 	}
 }
 
+static bool ntfs_file_name_attr_is_valid(const struct attr_record *a)
+{
+	const struct file_name_attr *fn;
+	u32 attr_len = le32_to_cpu(a->length);
+	u32 value_length;
+	u16 value_offset;
+	u32 file_name_size;
+
+	if (a->non_resident)
+		return false;
+
+	if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
+			sizeof(a->data.resident.reserved))
+		return false;
+
+	value_length = le32_to_cpu(a->data.resident.value_length);
+	value_offset = le16_to_cpu(a->data.resident.value_offset);
+
+	if (value_length > attr_len || value_offset > attr_len - value_length)
+		return false;
+
+	if (value_length < ntfs_resident_attr_min_value_length(AT_FILE_NAME))
+		return false;
+
+	fn = (const struct file_name_attr *)((const u8 *)a + value_offset);
+	file_name_size = fn->file_name_length * sizeof(__le16);
+
+	return file_name_size <=
+			value_length - offsetof(struct file_name_attr, file_name);
+}
+
 /*
  * ntfs_attr_find - find (next) attribute in mft record
  * @type:	attribute type to find
@@ -705,6 +736,13 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
+		if (a->type == AT_FILE_NAME &&
+		    !ntfs_file_name_attr_is_valid(a)) {
+			ntfs_error(vol->sb,
+				   "Corrupt $FILE_NAME attribute in MFT record %llu\n",
+				   ctx->ntfs_ino->mft_no);
+			break;
+		}
 		if (type == AT_UNUSED)
 			return 0;
 		if (a->type != type)
@@ -1252,6 +1290,14 @@ static int ntfs_external_attr_find(const __le32 type,
 
 		ctx->attr = a;
 
+		if (a->type == AT_FILE_NAME &&
+		    !ntfs_file_name_attr_is_valid(a)) {
+			ntfs_error(vol->sb,
+				   "Corrupt $FILE_NAME attribute in MFT record %llu\n",
+				   ctx->ntfs_ino->mft_no);
+			break;
+		}
+
 		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
-- 
2.43.0