[PATCH] adfs: bound big-directory entry name length to ADFS_FPLUS_NAME_LEN

[PATCH] adfs: bound big-directory entry name length to ADFS_FPLUS_NAME_LEN

[hyunjungg]

From: Hyunjung Ko <hj351016@gmail.com>

adfs_fplus_getnext() reads a directory entry's name length straight from
the on-disk big-directory entry and uses it, unchecked, as the length of
a copy into the fixed-size name buffer of the on-stack struct object_info:

	obj->name_len = le32_to_cpu(bde.bigdirobnamelen);

	offset = adfs_fplus_offset(h, le32_to_cpu(h->bigdirentries));
	offset += le32_to_cpu(bde.bigdirobnameptr);

	ret = adfs_dir_copyfrom(obj->name, dir, offset, obj->name_len);

obj->name is char[ADFS_MAX_NAME_LEN] (260 bytes), but bigdirobnamelen is
an attacker-controlled __le32 with no upper bound.  adfs_dir_copyfrom()
only validates that the source offset+length stay within the directory
buffers; it does not bound the length against the destination.  A crafted
image whose directory holds enough data at the name offset therefore lets
a single entry drive a copy of up to ~4 GiB into the 260-byte stack
buffer, a stack out-of-bounds write (detected by CONFIG_STACKPROTECTOR as
a corrupted canary, or by KASAN with CONFIG_KASAN_STACK).  The copy is
reached from both readdir and ->lookup via adfs_dir_lookup_byname().

The old-format directory parser (dir_f.c) already caps names at
ADFS_F_NAME_LEN while reading them; the big-directory parser has no
equivalent guard even though ADFS_FPLUS_NAME_LEN (255) is defined for
exactly this limit and is used elsewhere (super.c, s_namelen).

Reject entries whose name length exceeds ADFS_FPLUS_NAME_LEN before the
copy.  255 bytes fit in the 260-byte buffer and still leave room for the
4-byte ",xyz" filetype suffix that adfs_object_fixup() may append.

To trigger: build an ADFS image with a valid F+ ("big") directory whose
header, tail (BIGDIRENDNAME magic, matching masseq) and computed
check byte all validate, containing one adfs_bigdirentry with
bigdirobnamelen set to e.g. 300 and bigdirobnameptr pointing at 300 bytes
of in-bounds directory data.  mount -o loop followed by ls (or opening a
path under the mount) overflows obj->name.  The check byte is a computed
checksum, which is why random fuzzing rarely reaches this code; a
hand-crafted image does.

Reproduced under KASAN on 7.3-rc3 and 7.4-rc1 by mounting a crafted
ADFS image and running ls: KASAN reports a stack-out-of-bounds in the
adfs_fplus_getnext() name copy path.

Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 fs/adfs/dir_fplus.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/adfs/dir_fplus.c b/fs/adfs/dir_fplus.c
--- a/fs/adfs/dir_fplus.c
+++ b/fs/adfs/dir_fplus.c
@@ -191,6 +191,9 @@ adfs_fplus_getnext(struct adfs_dir *dir, struct object_info *obj)
 	obj->attr     = le32_to_cpu(bde.bigdirattr);
 	obj->name_len = le32_to_cpu(bde.bigdirobnamelen);

+	if (obj->name_len > ADFS_FPLUS_NAME_LEN)
+		return -EIO;
+
 	offset = adfs_fplus_offset(h, le32_to_cpu(h->bigdirentries));
 	offset += le32_to_cpu(bde.bigdirobnameptr);

--
2.43.0