[PATCH v2 2/3] ntfs: bound the look-ahead attribute-list entry in ntfs_external_attr_find()

[Bryam Vargas]

When resolving an attribute lookup with a non-zero @lowest_vcn,
ntfs_external_attr_find() peeks at the next $ATTRIBUTE_LIST entry to
decide whether to keep searching, but bounds that not-yet-validated
entry only with "(u8 *)next_al_entry + 6 < al_end" (which proves just
bytes 0..6 are in range) and "(u8 *)next_al_entry + length <= al_end"
with an attacker-controlled, non-8-aligned length. It then reads
next_al_entry->lowest_vcn (an __le64 at offset 8) and the name at
next_al_entry->name_offset, both of which can lie past al_end -- the
exact end of the kvmalloc'd attribute-list buffer (allocated at the
on-disk attr_list_size, no rounding). A crafted on-disk $ATTRIBUTE_LIST
whose last entry sits a few bytes before al_end therefore yields a slab
out-of-bounds read when the inode is read.

Apply to the look-ahead entry the same bounds the main loop already
applies to the current entry: require the fixed header up to
offsetof(struct attr_list_entry, name) to be in range, and the name to
lie within the buffer, before dereferencing lowest_vcn and the name.

Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
---
v2: dropped the redundant Reported-by; reproducer and KASAN splat omitted on
    the public list -- available to the maintainers on request. Posting per
    security@xxxxxxxxxx guidance that crafted-image filesystem bugs are out of
    the kernel security process's threat model.

Verified by mounting a crafted NTFS image under KASAN (fs/ntfs built as a
module against v7.1-rc5): a slab out-of-bounds read of next_al_entry->lowest_vcn
during inode read; a benign mkntfs image is KASAN-clean on the same kernel
(control). Arch-independent: the on-disk struct is __packed (lowest_vcn at
offset 8, fixed header up to offsetof(struct attr_list_entry, name) == 26) and
the over-read is an le64; identical geometry built -m32/-m64, so 32- and 64-bit
kernels are both affected.

Fixes note: this is in the fs/ntfs driver added during the v7.1 merge window.
The look-ahead expression is carried verbatim from the legacy fs/ntfs driver
(removed in v6.9; present unchanged since v2.6.12), where __ntfs_malloc()
rounded the list allocation up to a full kmalloc(PAGE_SIZE) and masked it; the
new driver's exact-size kvmalloc(attr_list_size) exposes it. Please add the
appropriate Fixes: tag for the commit that introduced ntfs_external_attr_find()
in the new driver.

 fs/ntfs/attrib.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/ntfs/attrib.c b/fs/ntfs/attrib.c
index 421c6cdcbb53..aed68b8e69ea 100644
--- a/fs/ntfs/attrib.c
+++ b/fs/ntfs/attrib.c
@@ -1137,9 +1137,14 @@ static int ntfs_external_attr_find(const __le32 type,
 		 * we have reached the right one or the search has failed.
 		 */
 		if (lowest_vcn && (u8 *)next_al_entry >= al_start &&
-				(u8 *)next_al_entry + 6 < al_end &&
+				(u8 *)next_al_entry +
+						offsetof(struct attr_list_entry, name) <= al_end &&
 				(u8 *)next_al_entry + le16_to_cpu(
 					next_al_entry->length) <= al_end &&
+				(!next_al_entry->name_length ||
+				 (u8 *)next_al_entry + next_al_entry->name_offset +
+				 next_al_entry->name_length * sizeof(__le16) <=
+						al_end) &&
 				le64_to_cpu(next_al_entry->lowest_vcn) <=
 					lowest_vcn &&
 				next_al_entry->type == al_entry->type &&
-- 
2.43.0