[PATCH xfs] xfs: validate remote attr value length in the leaf verifier

[Miao Zhao]

xfs_attr3_leaf_verify_entry() checks a remote attr entry's namelen and
valueblk but never its valuelen. A crafted image can set valuelen to a
value with the high bit set; xfs_attr3_leaf_getvalue() then stores it
in the signed int args->rmtvaluelen, where it goes negative. The
negative length bypasses the -ERANGE size check in xfs_attr_copy_value()
because the comparison is signed, and xfs_attr_rmtval_get() then
reinterprets it as a huge unsigned value, driving the memcpy loop in
xfs_attr_rmtval_copyout() past the end of the getxattr(2) result buffer
with image-controlled data (up to ~64KB on v5, where the remote block
header verifier caps rm_offset + rm_bytes at XFS_XATTR_SIZE_MAX;
unbounded on v4, where remote blocks carry no header at all).

Any unprivileged user with read access to the file can trigger this
with getxattr(2) once a crafted image is mounted, e.g. by a privileged
auto-mount daemon:

 BUG: KASAN: slab-out-of-bounds in xfs_attr_rmtval_copyout
 Write of size 4040 at addr 0000000061bf4ee0 by task xattr_tool/37
 CPU: 0 UID: 65534 PID: 37 Comm: xattr_tool Not tainted 7.2.0-rc5 #1
 Call Trace:
  memcpy
  xfs_attr_rmtval_copyout
  xfs_attr_rmtval_get
  xfs_attr_copy_value
  xfs_attr3_leaf_getvalue
  xfs_attr_node_get
  xfs_attr_get
  xfs_xattr_get
  vfs_getxattr
  sys_getxattr

The same unchecked valuelen is consumed on the INCOMPLETE sweep path
(xfs_attr_inactive.c) to size stale remote blocks, so validate it for
all remote entries: reject valuelen larger than XFS_XATTR_SIZE_MAX,
and reject a zero valuelen on complete entries (a value is only stored
remotely when it is too large for the local format, so zero is never
valid there; INCOMPLETE entries legitimately carry a zeroed valuelen
until the operation completes).

Fixes: c84760659dcf ("xfs: check attribute leaf block structure")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Miao Zhao <muel@xxxxxxxx>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 86c5c09a5db4..b8ae25be639a 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -336,8 +336,11 @@ xfs_attr3_leaf_verify_entry(
 		name_end = (char *)rentry + namesize;
 		if (rentry->namelen == 0)
 			return __this_address;
+		if (be32_to_cpu(rentry->valuelen) > XFS_XATTR_SIZE_MAX)
+			return __this_address;
 		if (!(ent->flags & XFS_ATTR_INCOMPLETE) &&
-		    rentry->valueblk == 0)
+		    (rentry->valueblk == 0 ||
+		     be32_to_cpu(rentry->valuelen) == 0))
 			return __this_address;
 	}
 
-- 
2.34.1