[PATCH v2] ntfs: fix u16 truncation of restart-area length check

Bryam Vargas posted 1 patch 1 day, 4 hours ago
fs/ntfs/logfile.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH v2] ntfs: fix u16 truncation of restart-area length check
Posted by Bryam Vargas 1 day, 4 hours ago
ntfs_check_restart_area() validates that the $LogFile restart area and
its trailing log client record array fit within the system page size:

	u16 ra_ofs, ra_len, ca_ofs;
	...
	ra_len = ca_ofs + le16_to_cpu(ra->log_clients) *
			sizeof(struct log_client_record);
	if (ra_ofs + ra_len > le32_to_cpu(rp->system_page_size) || ...)
		return false;

ra_len is u16, but the right-hand side is computed in size_t
(sizeof(struct log_client_record) == 160). Both ca_ofs and log_clients
come straight from the on-disk restart area. With an on-disk
log_clients of 410 the product 410 * 160 = 65600; adding ca_ofs and
storing into the u16 ra_len truncates modulo 65536 (e.g. ca_ofs 64
gives ra_len 128), so the "fits in the page" check passes even though
the client array described by log_clients extends far beyond the page.

ntfs_check_log_client_array() then walks the array bounded only by the
on-disk log_clients count:

	cr = ca + idx;
	if (cr->prev_client != LOGFILE_NO_CLIENT) ...

For log_clients 410 it dereferences records up to ca + 409 * 160,
~64 KiB past the kvzalloc(system_page_size) restart-page buffer -- an
out-of-bounds read of attacker-controlled extent, reachable when a
crafted NTFS image is mounted (load_and_check_logfile() at mount time).
This is the in-kernel analogue of CVE-2022-30789, fixed in the ntfs-3g
userspace driver but never in this revived classic driver.

Compute the restart-area length in a u32 so the existing bounds check
rejects an over-large client array instead of being defeated by the
truncation. Widen ra_ofs and ca_ofs to u32 as well: both are loaded
from __le16 on-disk fields and every comparison already promotes to
int/size_t, so this changes no result and keeps the declaration uniform.

Fixes: 1e9ea7e04472 ("Revert \"fs: Remove NTFS classic\"")
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
v2: widen all three locals (ra_ofs, ra_len, ca_ofs) to u32, not just
    ra_len, per David Laight's review. The fs_bits/ilog2() cleanup David
    suggested will follow as a separate patch (the current loop computes
    ilog2()+1, so it is not a drop-in). v1 went out quoted-printable via
    Proton Mail; this is resent as plain text.

Reproduced on a KASAN build (in-kernel ntfs, fresh-boot slab) by
mounting a crafted image whose $LogFile restart area sets
log_clients=410 (ra_len truncates to a value that passes the
system-page-size check):

  BUG: KASAN: slab-out-of-bounds in ntfs_check_logfile+0x1e52/0x2460 [ntfs]
  Read of size 2 ... by task mount
    (cr->prev_client, with ntfs_check_log_client_array() inlined)
    ntfs_fill_super -> ntfs_iget -> ntfs_check_logfile
  Allocated by task ...: __kasan_kmalloc -- the kvzalloc(system_page_size)
  restart-page buffer (the OOB read lands at buffer_base + system_page_size)

With this patch the same image is rejected by the existing
ntfs_check_restart_area() bound (ra_len is no longer truncated), so
ntfs_check_log_client_array() is never reached, no OOB access occurs,
and the mount fails over to read-only. A clean image is unaffected.
Full A/B logs available on request.

 fs/ntfs/logfile.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/ntfs/logfile.c b/fs/ntfs/logfile.c
--- a/fs/ntfs/logfile.c
+++ b/fs/ntfs/logfile.c
@@ -132,7 +132,7 @@ static bool ntfs_check_restart_area(struct inode *vi, struct restart_page_header *rp)
 {
 	u64 file_size;
 	struct restart_area *ra;
-	u16 ra_ofs, ra_len, ca_ofs;
+	u32 ra_ofs, ra_len, ca_ofs;
 	u8 fs_bits;

 	ntfs_debug("Entering.");
--
2.43.0