[Bug][KASAN] crash in xattr_getsecurity()

[Bug][KASAN] crash in xattr_getsecurity()

[Sachin Grover]

[-- Attachment #1: Type: text/plain, Size: 3227 bytes --]

Hi,

Kernel panic is coming on calling lgetxattr() sys api with random user
space value.

[   25.833951] Call trace:
[   25.833954] [<ffffff86adc8af40>] dump_backtrace+0x0/0x2a8
[   25.833957] [<ffffff86adc8b484>] show_stack+0x20/0x28
[   25.833959] [<ffffff86ae02b744>] dump_stack+0xa8/0xe0
[   25.833962] [<ffffff86ade79ed0>] xattr_getsecurity+0xac/0xd4
[   25.833964] [<ffffff86ade79f90>] vfs_getxattr+0x98/0xcc
[   25.833966] [<ffffff86ade7a548>] getxattr+0x9c/0x1d4
[   25.833969] [<ffffff86ade7a6f4>] path_getxattr+0x74/0xc4
[   25.833971] [<ffffff86ade7afd8>] SyS_lgetxattr+0x3c/0x48
[   25.833973] [<ffffff86adc83770>] el0_svc_naked+0x24/0x28

setxattr() is getting size and value from the userspace, if I am giving
size as 64840, my code is entering this part and crashing on doing memcpy
under  xattr_getsecurity().

rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9>
= string_to_context_struct(&policydb, &sidtab, scontext2,
				      scontext_len
<http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>,
&context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>,
def_sid <http://opengrok.qualcomm.com/source/s?defs=def_sid&project=kernel_msm-4.9>);
	*if* (rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9>
== -EINVAL <http://opengrok.qualcomm.com/source/s?defs=EINVAL&project=kernel_msm-4.9>
&& force <http://opengrok.qualcomm.com/source/s?defs=force&project=kernel_msm-4.9>)
{
	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.str
= str;
	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.len
<http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9>
= scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>;
		str = NULL <http://opengrok.qualcomm.com/source/s?defs=NULL&project=kernel_msm-4.9>;



//rc value is coming as EINVAL(-22). In pass case rc value is always 0.


Please let me know if this fix is good enough.


rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9>
= string_to_context_struct(&policydb, &sidtab, scontext2,
				      scontext_len
<http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>,
&context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>,
def_sid <http://opengrok.qualcomm.com/source/s?defs=def_sid&project=kernel_msm-4.9>);
	*if* (rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9>
== -EINVAL <http://opengrok.qualcomm.com/source/s?defs=EINVAL&project=kernel_msm-4.9>
&& force <http://opengrok.qualcomm.com/source/s?defs=force&project=kernel_msm-4.9>)
{
	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.str
= str;
-      context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.len
<http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9>
= scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>;

+      context.len = strlen(str);

		str = NULL <http://opengrok.qualcomm.com/source/s?defs=NULL&project=kernel_msm-4.9>;


Regards,

Sachin Grover

[-- Attachment #2: Type: text/html, Size: 10713 bytes --]

Re: [Bug][KASAN] crash in xattr_getsecurity()

[Sachin Grover]

[-- Attachment #1: Type: text/plain, Size: 4248 bytes --]

To clarify more, data and size both are coming from userspace, so I sent a
string in void * __user arg of  lgetxattr(). My string format is -
"abcdef/0ashasalksjas"
i.e. I sent a string with null character in between.
I sent size as 64840.
 Now according to your change:

context.str = str;               //context.str= "abcdef/0ashasalksjas"
context.len <http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9>
= scontext_len;      //context.len
<http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9>
= 64840


But the actual length of string was 6. Because of not taking the
actual length into consideration, memcpy function is failing in
xattr_getsecurity() func.


I am not the expert in selinux stack so want your input on this.




On Thu, May 24, 2018 at 11:42 AM, Sachin Grover <sachin.grover91@gmail.com>
wrote:

> Hi,
>
> Kernel panic is coming on calling lgetxattr() sys api with random user
> space value.
>
> [   25.833951] Call trace:
> [   25.833954] [<ffffff86adc8af40>] dump_backtrace+0x0/0x2a8
> [   25.833957] [<ffffff86adc8b484>] show_stack+0x20/0x28
> [   25.833959] [<ffffff86ae02b744>] dump_stack+0xa8/0xe0
> [   25.833962] [<ffffff86ade79ed0>] xattr_getsecurity+0xac/0xd4
> [   25.833964] [<ffffff86ade79f90>] vfs_getxattr+0x98/0xcc
> [   25.833966] [<ffffff86ade7a548>] getxattr+0x9c/0x1d4
> [   25.833969] [<ffffff86ade7a6f4>] path_getxattr+0x74/0xc4
> [   25.833971] [<ffffff86ade7afd8>] SyS_lgetxattr+0x3c/0x48
> [   25.833973] [<ffffff86adc83770>] el0_svc_naked+0x24/0x28
>
> setxattr() is getting size and value from the userspace, if I am giving
> size as 64840, my code is entering this part and crashing on doing memcpy
> under  xattr_getsecurity().
>
> rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> = string_to_context_struct(&policydb, &sidtab, scontext2,
> 				      scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>, &context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>, def_sid <http://opengrok.qualcomm.com/source/s?defs=def_sid&project=kernel_msm-4.9>);
> 	*if* (rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> == -EINVAL <http://opengrok.qualcomm.com/source/s?defs=EINVAL&project=kernel_msm-4.9> && force <http://opengrok.qualcomm.com/source/s?defs=force&project=kernel_msm-4.9>) {
> 	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.str = str;
> 	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.len <http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9> = scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>;
> 		str = NULL <http://opengrok.qualcomm.com/source/s?defs=NULL&project=kernel_msm-4.9>;
>
>
>
> //rc value is coming as EINVAL(-22). In pass case rc value is always 0.
>
>
> Please let me know if this fix is good enough.
>
>
> rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> = string_to_context_struct(&policydb, &sidtab, scontext2,
> 				      scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>, &context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>, def_sid <http://opengrok.qualcomm.com/source/s?defs=def_sid&project=kernel_msm-4.9>);
> 	*if* (rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> == -EINVAL <http://opengrok.qualcomm.com/source/s?defs=EINVAL&project=kernel_msm-4.9> && force <http://opengrok.qualcomm.com/source/s?defs=force&project=kernel_msm-4.9>) {
> 	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.str = str;
> -      context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.len <http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9> = scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>;
>
> +      context.len = strlen(str);
>
> 		str = NULL <http://opengrok.qualcomm.com/source/s?defs=NULL&project=kernel_msm-4.9>;
>
>
> Regards,
>
> Sachin Grover
>
>
>

[-- Attachment #2: Type: text/html, Size: 15378 bytes --]

Re: [Bug][KASAN] crash in xattr_getsecurity()

[Stephen Smalley]

On 05/24/2018 02:12 AM, Sachin Grover wrote:
> Hi,
> 
> Kernel panic is coming on calling lgetxattr() sys api with random user space value.
> 
> [   25.833951] Call trace:
> [   25.833954] [<ffffff86adc8af40>] dump_backtrace+0x0/0x2a8
> [   25.833957] [<ffffff86adc8b484>] show_stack+0x20/0x28
> [   25.833959] [<ffffff86ae02b744>] dump_stack+0xa8/0xe0
> [   25.833962] [<ffffff86ade79ed0>] xattr_getsecurity+0xac/0xd4
> [   25.833964] [<ffffff86ade79f90>] vfs_getxattr+0x98/0xcc
> [   25.833966] [<ffffff86ade7a548>] getxattr+0x9c/0x1d4
> [   25.833969] [<ffffff86ade7a6f4>] path_getxattr+0x74/0xc4
> [   25.833971] [<ffffff86ade7afd8>] SyS_lgetxattr+0x3c/0x48
> [   25.833973] [<ffffff86adc83770>] el0_svc_naked+0x24/0x28
> 
> setxattr() is getting size and value from the userspace, if I am giving size as 64840, my code is entering this part and crashing on doing memcpy under  xattr_getsecurity().
> 
> rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> = string_to_context_struct(&policydb, &sidtab, scontext2,
> 				      scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>, &context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>, def_sid <http://opengrok.qualcomm.com/source/s?defs=def_sid&project=kernel_msm-4.9>);
> 	*if* (rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> == -EINVAL <http://opengrok.qualcomm.com/source/s?defs=EINVAL&project=kernel_msm-4.9> && force <http://opengrok.qualcomm.com/source/s?defs=force&project=kernel_msm-4.9>) {
> 	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.str = str;
> 	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.len <http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9> = scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>;
> 		str = NULL <http://opengrok.qualcomm.com/source/s?defs=NULL&project=kernel_msm-4.9>;
> 
> 
> 
> //rc value is coming as EINVAL(-22). In pass case rc value is always 0.
> 
> 
> Please let me know if this fix is good enough.
> 
> 
> rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> = string_to_context_struct(&policydb, &sidtab, scontext2,
> 				      scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>, &context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>, def_sid <http://opengrok.qualcomm.com/source/s?defs=def_sid&project=kernel_msm-4.9>);
> 	*if* (rc <http://opengrok.qualcomm.com/source/s?defs=rc&project=kernel_msm-4.9> == -EINVAL <http://opengrok.qualcomm.com/source/s?defs=EINVAL&project=kernel_msm-4.9> && force <http://opengrok.qualcomm.com/source/s?defs=force&project=kernel_msm-4.9>) {
> 	context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.str = str;
> -      context <http://opengrok.qualcomm.com/source/s?defs=context&project=kernel_msm-4.9>.len <http://opengrok.qualcomm.com/source/s?defs=len&project=kernel_msm-4.9> = scontext_len <http://opengrok.qualcomm.com/source/s?defs=scontext_len&project=kernel_msm-4.9>;
> 
> +      context.len = strlen(str);
> 
> 		str = NULL <http://opengrok.qualcomm.com/source/s?defs=NULL&project=kernel_msm-4.9>;

IIUC, this is only possible if a process with CAP_MAC_ADMIN and that is allowed mac_admin permission in SELinux policy (or if SELinux is permissive) sets a security.selinux xattr with an embedded NUL on a file and then it or any other process performs a getxattr() on that file with a length greater than the length of the string up to the embedded NUL.  Is that accurate?  If so, then this is never possible on Android (no process is allowed mac_admin permission and SELinux is always enforcing) and is only possible in Fedora/RHEL for a few specific root/CAP_MAC_ADMIN processes in specific SELinux domains (sesearch -A -p mac_admin) if SELinux is enforcing or any root/CAP_MAC_ADMIN process if SELinux is permissive.  Regardless, not exploitable without CAP_MAC_ADMIN.  Also possible with a crafted filesystem image that already contains such an xattr.

Generally we include the terminating NUL in the length, so context.len would be strlen(str)+1.  Otherwise, I think your fix is reasonable.  I think this bug goes back to 9a59daa03df72526d234b91dd3e32ded5aebd3ef ("SELinux: fix sleeping allocation in security_context_to_sid").