[PATCH v2] ntfs: validate resident attribute values on lookup

[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 variable-length resident
formats, however, the fixed part is not enough: embedded length fields
can still point callers past the resident value.

A crafted image can set a small resident $FILE_NAME 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. The stack included
ntfs_lookup(), ntfs_iget(), ntfs_read_locked_inode(), ntfs_attr_name_get(),
ntfs_ucstonls(), and utf16s_to_utf8s().

Add a shared resident attribute value validator and use it before a
lookup path can return an attribute, including the AT_UNUSED enumeration
case where callers inspect returned attributes directly. The helper
validates resident value bounds, minimum value sizes, and variable-length
$FILE_NAME and $INDEX_ROOT fields, while preserving the fixed-size
checks for attributes such as $VOLUME_INFORMATION and $EA_INFORMATION.
Log corruption at the rejection point with the attribute name where
known.

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

Type-specific format validation for $VOLUME_NAME is intentionally not
added to this helper, even though $VOLUME_NAME is a variable-length
resident attribute. Existing callers such as ntfs_write_volume_label()
treat a failed AT_VOLUME_NAME lookup as an absent label and
unconditionally add a new record afterwards. If the helper rejected
odd-length labels at lookup time, the corrupt record would stay in
place and the caller would append a new record next to it, producing
two $VOLUME_NAME attributes on disk. Caller-side cleanup must precede
any lookup-time rejection of malformed $VOLUME_NAME payloads, so the
type-specific check is left for a follow-up patch. The generic resident
bounds checks (value_offset / value_length / attr_len) still apply to
$VOLUME_NAME, and its entry in the type-name table is kept so that
those failures log with the attribute name.

Fixes: 6ceb4cc81ef3 ("ntfs: add bound checking to ntfs_attr_find")
Signed-off-by: DaeMyung Kang <charsyam@xxxxxxxxx>
---
Patch 1/2 of the previous series ("ntfs: free link name from
ntfs_name_cache") has been applied to ntfs-next; this v2 contains only
the remaining patch, with the resident attribute validation reworked per
review feedback.

Changes since v1:
- Refactor resident attribute validation into a shared helper
  ntfs_attr_value_is_valid() used by both ntfs_attr_find() and
  ntfs_external_attr_find(), and remove the duplicated inline check
  from ntfs_external_attr_find().
- Split out resident header/value bounds extraction into
  ntfs_resident_attr_value_get(), returning a small view struct so the
  top-level orchestration reads as: get view -> common min check ->
  per-type format check.
- Validate before the AT_UNUSED enumeration path in ntfs_attr_find()
  can return the attribute to callers that inspect it directly.
- Reject non-resident $FILE_NAME records: the format requires
  $FILE_NAME to be resident and callers treat returned records as
  resident.
- Extend type-specific validation beyond $FILE_NAME to $INDEX_ROOT
  (entries_offset / index_length / allocated_size bounds and 8-byte
  alignment); add $INDEX_ROOT to ntfs_resident_attr_min_value_length().
- Type-specific format validation for $VOLUME_NAME is intentionally
  not added; see the commit message for the caller-side reason.
  Generic resident bounds checks still apply, and the $VOLUME_NAME
  entry in the type-name table is kept so that those failures log
  with a readable attribute name.
- Log corruption at the rejection point with the attribute type name
  via a new ntfs_attr_type_name() helper, matching the existing
  attribute-name corruption message in ntfs_attr_find().

 fs/ntfs/attrib.c | 182 +++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 151 insertions(+), 31 deletions(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..63b3b0d13c45 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -588,6 +588,8 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 			sizeof(__le16) * 1;
 	case AT_VOLUME_INFORMATION:
 		return sizeof(struct volume_information);
+	case AT_INDEX_ROOT:
+		return sizeof(struct index_root);
 	case AT_EA_INFORMATION:
 		return sizeof(struct ea_information);
 	default:
@@ -595,6 +597,144 @@ static u32 ntfs_resident_attr_min_value_length(const __le32 type)
 	}
 }
 
+static const char *ntfs_attr_type_name(const __le32 type)
+{
+	switch (type) {
+	case AT_STANDARD_INFORMATION:
+		return "$STANDARD_INFORMATION";
+	case AT_FILE_NAME:
+		return "$FILE_NAME";
+	case AT_VOLUME_NAME:
+		return "$VOLUME_NAME";
+	case AT_VOLUME_INFORMATION:
+		return "$VOLUME_INFORMATION";
+	case AT_INDEX_ROOT:
+		return "$INDEX_ROOT";
+	case AT_EA_INFORMATION:
+		return "$EA_INFORMATION";
+	default:
+		return NULL;
+	}
+}
+
+static bool ntfs_file_name_attr_value_is_valid(const u8 *value, const u32 value_length)
+{
+	const struct file_name_attr *fn;
+	u32 file_name_size;
+
+	if (value_length < ntfs_resident_attr_min_value_length(AT_FILE_NAME))
+		return false;
+
+	fn = (const struct file_name_attr *)value;
+	file_name_size = fn->file_name_length * sizeof(__le16);
+
+	return file_name_size <=
+			value_length - offsetof(struct file_name_attr, file_name);
+}
+
+static bool ntfs_index_root_attr_value_is_valid(const u8 *value, const u32 value_length)
+{
+	const struct index_root *ir;
+	u32 index_size;
+	u32 entries_offset;
+	u32 index_length;
+	u32 allocated_size;
+
+	if (value_length < ntfs_resident_attr_min_value_length(AT_INDEX_ROOT))
+		return false;
+
+	ir = (const struct index_root *)value;
+	index_size = value_length - offsetof(struct index_root, index);
+	entries_offset = le32_to_cpu(ir->index.entries_offset);
+	index_length = le32_to_cpu(ir->index.index_length);
+	allocated_size = le32_to_cpu(ir->index.allocated_size);
+
+	if ((entries_offset | index_length | allocated_size) & 7 ||
+	    entries_offset < sizeof(struct index_header) ||
+	    entries_offset > index_length ||
+	    index_length > allocated_size ||
+	    allocated_size > index_size ||
+	    index_length - entries_offset < sizeof(struct index_entry_header))
+		return false;
+
+	return true;
+}
+
+struct ntfs_resident_attr_value {
+	const u8 *data;
+	u32 len;
+};
+
+static bool ntfs_resident_attr_value_get(const struct attr_record *a,
+					 struct ntfs_resident_attr_value *value)
+{
+	u32 attr_len;
+	u16 value_offset;
+
+	attr_len = le32_to_cpu(a->length);
+	if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
+			sizeof(a->data.resident.reserved))
+		return false;
+
+	value->len = le32_to_cpu(a->data.resident.value_length);
+	value_offset = le16_to_cpu(a->data.resident.value_offset);
+
+	if (value->len > attr_len || value_offset > attr_len - value->len)
+		return false;
+
+	value->data = (const u8 *)a + value_offset;
+	return true;
+}
+
+static bool ntfs_attr_value_is_valid(struct ntfs_volume *vol,
+				     const struct attr_record *a,
+				     const u64 mft_no)
+{
+	struct ntfs_resident_attr_value value;
+	const char *type_name;
+	u32 min_len;
+
+	if (a->non_resident) {
+		if (a->type != AT_FILE_NAME)
+			return true;
+		ntfs_error(vol->sb,
+			   "Corrupt non-resident $FILE_NAME attribute in MFT record %llu\n",
+			   mft_no);
+		return false;
+	}
+
+	if (!ntfs_resident_attr_value_get(a, &value))
+		goto corrupt;
+
+	min_len = ntfs_resident_attr_min_value_length(a->type);
+	if (min_len && value.len < min_len)
+		goto corrupt;
+
+	switch (a->type) {
+	case AT_FILE_NAME:
+		if (!ntfs_file_name_attr_value_is_valid(value.data, value.len))
+			goto corrupt;
+		break;
+	case AT_INDEX_ROOT:
+		if (!ntfs_index_root_attr_value_is_valid(value.data, value.len))
+			goto corrupt;
+		break;
+	}
+	return true;
+
+corrupt:
+	type_name = ntfs_attr_type_name(a->type);
+	if (type_name)
+		ntfs_error(vol->sb,
+			   "Corrupt resident %s attribute in MFT record %llu\n",
+			   type_name, mft_no);
+	else
+		ntfs_error(vol->sb,
+			   "Corrupt resident %#x attribute in MFT record %llu\n",
+			   le32_to_cpu(a->type), mft_no);
+	return false;
+}
+
 /*
  * ntfs_attr_find - find (next) attribute in mft record
  * @type:	attribute type to find
@@ -705,8 +845,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
-		if (type == AT_UNUSED)
+		if (type == AT_UNUSED) {
+			if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+				break;
 			return 0;
+		}
 		if (a->type != type)
 			continue;
 		/*
@@ -747,24 +890,11 @@ static int ntfs_attr_find(const __le32 type, const __le16 *name,
 			}
 		}
 
-		 /* Validate attribute's value offset/length */
-		if (!a->non_resident) {
-			u32 min_len;
-			u32 value_length = le32_to_cpu(a->data.resident.value_length);
-			u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
-
-			if (value_length > le32_to_cpu(a->length) ||
-			    value_offset > le32_to_cpu(a->length) - value_length)
-				break;
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
 
-			min_len = ntfs_resident_attr_min_value_length(a->type);
-			if (min_len && value_length < min_len) {
-				ntfs_error(vol->sb,
-					   "Too small %#x resident attribute value in MFT record %lld\n",
-					   le32_to_cpu(a->type), (long long)ctx->ntfs_ino->mft_no);
-				break;
-			}
-		} else {
+		/* Validate non-resident mapping-pairs fields. */
+		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
 
@@ -1252,6 +1382,9 @@ static int ntfs_external_attr_find(const __le32 type,
 
 		ctx->attr = a;
 
+		if (!ntfs_attr_value_is_valid(vol, a, ctx->ntfs_ino->mft_no))
+			break;
+
 		if (a->non_resident) {
 			u32 min_len;
 			u16 mp_offset;
@@ -1279,19 +1412,6 @@ static int ntfs_external_attr_find(const __le32 type,
 			u32 value_length = le32_to_cpu(a->data.resident.value_length);
 			u16 value_offset = le16_to_cpu(a->data.resident.value_offset);
 
-			if (attr_len < offsetof(struct attr_record, data.resident.reserved) +
-					sizeof(a->data.resident.reserved))
-				break;
-			if (value_length > attr_len || value_offset > attr_len - value_length)
-				break;
-
-			value_length = ntfs_resident_attr_min_value_length(a->type);
-			if (value_length && le32_to_cpu(a->data.resident.value_length) <
-			    value_length) {
-				pr_err("Too small resident attribute value in MFT record %lld, type %#x\n",
-				       (long long)ctx->ntfs_ino->mft_no, a->type);
-				break;
-			}
 			if (value_length == val_len &&
 			    !memcmp((u8 *)a + value_offset, val, val_len)) {
 attr_found:
-- 
2.43.0