fs/ntfs3: validate that EA entry size covers its name and value
Subsystems (2)
PATCH 1/1 fs/ntfs3: validate that EA entry size covers its name and value
From: Aldo Ariel Panzardo <[email protected]>
ntfs_read_ea() walks the on-disk $EA attribute and checks each EA_FULL entry for consistency. For an entry with a non-zero ef->size (the offset to the next entry) the loop only verifies that this stride fits in the remaining buffer, and then continues:
if (ef->size) {
ea_size = le32_to_cpu(ef->size);
if (ea_size > bytes)
goto out1;
continue;
}It never verifies that the entry's own name (ef->name_len) and value (ef->elength) actually fit inside ef->size. A later reader trusts ef->elength unconditionally; ntfs_get_ea() does:
len = le16_to_cpu(ea->elength); /* up to 0xffff */ ... if (len > size) /* size is the user buffer */ return -ERANGE; memcpy(buffer, ea->name + ea->name_len + 1, len);
A crafted image with a small but valid ef->size (e.g. 24) and ef->elength == 0xffff therefore passes validation, and a getxattr() with a large enough user buffer copies up to 64 KiB starting just past the short entry -- an out-of-bounds read of the ea_all allocation that leaks adjacent kernel heap memory to userspace.
BUG: KASAN: slab-out-of-bounds in ntfs_get_ea+0x2b8/0x3f0 Read of size 65535 ... ntfs_get_ea -> ntfs_getxattr -> vfs_getxattr -> do_getxattr
The zero-ef->size branch already computes the same struct_size() to derive the stride; mirror that in the non-zero branch and reject the entry when its declared size cannot hold the name and value.
--- fs/ntfs3/xattr.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
fs/ntfs3/xattr.c +18 -0
@@ -155,6 +155,24 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea, ea_size =le32_to_cpu (ef->size);if (ea_size > bytes) goto out1; + +/* Check if we can use fields ef->name_len and ef->elength. */ +if (bytes < offsetof(struct EA_FULL , name)) + goto out1; + +/* +* The declared entry size (ef->size) must be large +* enough to hold the name and value. Otherwise a later +* reader such as ntfs_get_ea() copies ef->elength bytes +* starting past the entry, reading out of bounds of the +* ea buffer and leaking adjacent heap memory to +* userspace via getxattr(). +*/ +if (struct_size (ef, name, +1 + ef->name_len + +le16_to_cpu (ef->elength)) > + ea_size) + goto out1;continue ; }
Signature · 1 line
2.43.0
Konstantin Komarov
maintainerQuoted 75 lines · Your Name
From: Aldo Ariel Panzardo <[email protected]>
ntfs_read_ea()walks the on-disk $EA attribute and checks eachEA_FULLentry for consistency. For an entry with a non-zeroef->size(the offset to the next entry) the loop only verifies that this stride fits in the remaining buffer, and then continues:if (ef->size) { ea_size = le32_to_cpu(ef->size); if (ea_size > bytes) goto out1; continue; }It never verifies that the entry's own name (
ef->name_len) and value (ef->elength) actually fit insideef->size. A later reader trustsef->elengthunconditionally;ntfs_get_ea()does:len = le16_to_cpu(ea->elength); /* up to 0xffff */ ... if (len > size) /* size is the user buffer */ return -ERANGE; memcpy(buffer, ea->name + ea->name_len + 1, len);A crafted image with a small but valid
ef->size(e.g. 24) andef->elength==0xfffftherefore passes validation, and agetxattr()with a large enough user buffer copies up to 64 KiB starting just past the short entry -- an out-of-bounds read of theea_allallocation that leaks adjacent kernel heap memory to userspace.BUG: KASAN: slab-out-of-bounds in ntfs_get_ea+0x2b8/0x3f0 Read of size 65535 ... ntfs_get_ea -> ntfs_getxattr -> vfs_getxattr -> do_getxattrThe zero-ef->size branch already computes the same
struct_size()to derive the stride; mirror that in the non-zero branch and reject the entry when its declared size cannot hold the name and value.Fixes 0e8235d28f3a ("fs/ntfs3: Check fields while reading")Signed-off-by Aldo Ariel Panzardo <[email protected]>--- fs/ntfs3/xattr.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)fs/ntfs3/xattr.c +18
-1@@ -155,6 +155,24 @@ static int ntfs_read_ea(struct ntfs_inode *ni, struct EA_FULL **ea, ea_size =le32_to_cpu (ef->size);if (ea_size > bytes) goto out1; + +/* Check if we can use fields ef->name_len and ef->elength. */ +if (bytes < offsetof(struct EA_FULL , name)) + goto out1; + +/* +* The declared entry size (ef->size) must be large +* enough to hold the name and value. Otherwise a later +* reader such as ntfs_get_ea() copies ef->elength bytes +* starting past the entry, reading out of bounds of the +* ea buffer and leaking adjacent heap memory to +* userspace via getxattr(). +*/ +if (struct_size (ef, name, +1 + ef->name_len + +le16_to_cpu (ef->elength)) > + ea_size) + goto out1;continue ; } --
Quoted 1 line
2.43.0
Hello,
Your patch was applied, thank you.
Regards,
Konstantin