Skip to content

Commit

Permalink
capabilities: fix buffer overread on very short xattr
Browse files Browse the repository at this point in the history
If userspace attempted to set a "security.capability" xattr shorter than
4 bytes (e.g. 'setfattr -n security.capability -v x file'), then
cap_convert_nscap() read past the end of the buffer containing the xattr
value because it accessed the ->magic_etc field without verifying that
the xattr value is long enough to contain that field.

Fix it by validating the xattr value size first.

This bug was found using syzkaller with KASAN.  The KASAN report was as
follows (cleaned up slightly):

    BUG: KASAN: slab-out-of-bounds in cap_convert_nscap+0x514/0x630 security/commoncap.c:498
    Read of size 4 at addr ffff88002d8741c0 by task syz-executor1/2852

    CPU: 0 PID: 2852 Comm: syz-executor1 Not tainted 4.15.0-rc6-00200-gcc0aac99d977 torvalds#253
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
    Call Trace:
     __dump_stack lib/dump_stack.c:17 [inline]
     dump_stack+0xe3/0x195 lib/dump_stack.c:53
     print_address_description+0x73/0x260 mm/kasan/report.c:252
     kasan_report_error mm/kasan/report.c:351 [inline]
     kasan_report+0x235/0x350 mm/kasan/report.c:409
     cap_convert_nscap+0x514/0x630 security/commoncap.c:498
     setxattr+0x2bd/0x350 fs/xattr.c:446
     path_setxattr+0x168/0x1b0 fs/xattr.c:472
     SYSC_setxattr fs/xattr.c:487 [inline]
     SyS_setxattr+0x36/0x50 fs/xattr.c:483
     entry_SYSCALL_64_fastpath+0x18/0x85

Fixes: 8db6c34 ("Introduce v3 namespaced file capabilities")
Cc: <[email protected]> # v4.14+
Signed-off-by: Eric Biggers <[email protected]>
  • Loading branch information
ebiggers authored and 0day robot committed Jan 2, 2018
1 parent 464e1d5 commit a90562f
Showing 1 changed file with 9 additions and 12 deletions.
21 changes: 9 additions & 12 deletions security/commoncap.c
Original file line number Diff line number Diff line change
Expand Up @@ -348,21 +348,18 @@ static __u32 sansflags(__u32 m)
return m & ~VFS_CAP_FLAGS_EFFECTIVE;
}

static bool is_v2header(size_t size, __le32 magic)
static bool is_v2header(size_t size, const struct vfs_cap_data *cap)
{
__u32 m = le32_to_cpu(magic);
if (size != XATTR_CAPS_SZ_2)
return false;
return sansflags(m) == VFS_CAP_REVISION_2;
return sansflags(le32_to_cpu(cap->magic_etc)) == VFS_CAP_REVISION_2;
}

static bool is_v3header(size_t size, __le32 magic)
static bool is_v3header(size_t size, const struct vfs_cap_data *cap)
{
__u32 m = le32_to_cpu(magic);

if (size != XATTR_CAPS_SZ_3)
return false;
return sansflags(m) == VFS_CAP_REVISION_3;
return sansflags(le32_to_cpu(cap->magic_etc)) == VFS_CAP_REVISION_3;
}

/*
Expand Down Expand Up @@ -405,15 +402,15 @@ int cap_inode_getsecurity(struct inode *inode, const char *name, void **buffer,

fs_ns = inode->i_sb->s_user_ns;
cap = (struct vfs_cap_data *) tmpbuf;
if (is_v2header((size_t) ret, cap->magic_etc)) {
if (is_v2header((size_t) ret, cap)) {
/* If this is sizeof(vfs_cap_data) then we're ok with the
* on-disk value, so return that. */
if (alloc)
*buffer = tmpbuf;
else
kfree(tmpbuf);
return ret;
} else if (!is_v3header((size_t) ret, cap->magic_etc)) {
} else if (!is_v3header((size_t) ret, cap)) {
kfree(tmpbuf);
return -EINVAL;
}
Expand Down Expand Up @@ -470,9 +467,9 @@ static kuid_t rootid_from_xattr(const void *value, size_t size,
return make_kuid(task_ns, rootid);
}

static bool validheader(size_t size, __le32 magic)
static bool validheader(size_t size, const struct vfs_cap_data *cap)
{
return is_v2header(size, magic) || is_v3header(size, magic);
return is_v2header(size, cap) || is_v3header(size, cap);
}

/*
Expand All @@ -495,7 +492,7 @@ int cap_convert_nscap(struct dentry *dentry, void **ivalue, size_t size)

if (!*ivalue)
return -EINVAL;
if (!validheader(size, cap->magic_etc))
if (!validheader(size, cap))
return -EINVAL;
if (!capable_wrt_inode_uidgid(inode, CAP_SETFCAP))
return -EPERM;
Expand Down

0 comments on commit a90562f

Please sign in to comment.