linux-kernel - Re: [PATCH] selinux: KASAN: slab-out-of-bounds in xattr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHC9VhQui+U1wzms2=XSW7MeRZg-faAzZyG8XU8wc+bWAoVzJg@mail.gmail.com>
Date:   Wed, 30 May 2018 11:19:42 -0400
From:   Paul Moore <paul@...l-moore.com>
To:     Sachin Grover <sgrover@...eaurora.org>
Cc:     Stephen Smalley <sds@...ho.nsa.gov>,
        linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org, selinux@...ho.nsa.gov
Subject: Re: [PATCH] selinux: KASAN: slab-out-of-bounds in xattr_getsecurity

On Fri, May 25, 2018 at 4:31 AM, Sachin Grover <sgrover@...eaurora.org> wrote:
> Call trace:
>  [<ffffff9203a8d7a8>] dump_backtrace+0x0/0x428
>  [<ffffff9203a8dbf8>] show_stack+0x28/0x38
>  [<ffffff920409bfb8>] dump_stack+0xd4/0x124
>  [<ffffff9203d187e8>] print_address_description+0x68/0x258
>  [<ffffff9203d18c00>] kasan_report.part.2+0x228/0x2f0
>  [<ffffff9203d1927c>] kasan_report+0x5c/0x70
>  [<ffffff9203d1776c>] check_memory_region+0x12c/0x1c0
>  [<ffffff9203d17cdc>] memcpy+0x34/0x68
>  [<ffffff9203d75348>] xattr_getsecurity+0xe0/0x160
>  [<ffffff9203d75490>] vfs_getxattr+0xc8/0x120
>  [<ffffff9203d75d68>] getxattr+0x100/0x2c8
>  [<ffffff9203d76fb4>] SyS_fgetxattr+0x64/0xa0
>  [<ffffff9203a83f70>] el0_svc_naked+0x24/0x28
>
> If user get root access and calls security.selinux setxattr() with an
> embedded NUL on a file and then if some process performs a getxattr()
> on that file with a length greater than the actual length of the string,
> it would result in a panic.
>
> To fix this, add the actual length of the string to the security context
> instead of the length passed by the userspace process.
>
> Signed-off-by: Sachin Grover <sgrover@...eaurora.org>
> ---
>  security/selinux/ss/services.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks for reporting this and providing a patch.  It's small enough,
and passes all the regular tests, so I've merged it into
selinux/stable-4.17 (adding the stable metadata) and I'm going to send
it up to Linus today.

If Linus doesn't pull the fix in time for v4.17 I'll send it up during
the upcoming merge window.

> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 66ea81c..d17f5b4 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1434,7 +1434,7 @@ static int security_context_to_sid_core(const char *scontext, u32 scontext_len,
>                                       scontext_len, &context, def_sid);
>         if (rc == -EINVAL && force) {
>                 context.str = str;
> -               context.len = scontext_len;
> +               context.len = strlen(str) + 1;
>                 str = NULL;
>         } else if (rc)
>                 goto out_unlock;
> --
> 1.9.1

-- 
paul moore
www.paul-moore.com