[PATCH] umask(077) as a workaround for security vuln [mdev]

[Piotr Karbowski]

On 12/21/2012 04:09 PM, Piotr Karbowski wrote:
> On 12/18/2012 10:41 AM, Piotr Karbowski wrote:
>> On 12/18/2012 10:11 AM, Piotr Karbowski wrote:
>>> Hi,
>>>
>>> For some reason when i do move /dev/msr[0-9]+ nodes to /dev/cpu the
>>> /dev/cpu is getting 777 permissions. The very rule that affect it:
>>>
>>> msr([0-9]+)        root:root 600 =cpu/%1/msr
>>>
>>> Testcase:
>>>
>>> ### mdev.conf without msr:
>>> microcode       root:root 600 =cpu/
>>> cpu([0-9]+)     root:root 600 =cpu/%1/cpuid
>>>
>>> # rm /dev/cpu -rf; mdev -s; ls -ld /dev/cpu
>>> drwxr-xr-x 6 root root 140 Dec 18 10:07 /dev/cpu/
>>>
>>> Good so far.
>>>
>>> ### mdev.conf with msr:
>>> microcode       root:root 600 =cpu/
>>> cpu([0-9]+)     root:root 600 =cpu/%1/cpuid
>>> msr([0-9]+)     root:root 600 =cpu/%1/msr
>>>
>>> # rm /dev/cpu -rf; mdev -s; ls -ld /dev/cpu
>>> drwxrwxrwx 6 root root 140 Dec 18 10:08 /dev/cpu/
>>>
>>> And the /dev/cpu is 777, whatever is the order of msr, or even with
>>> there is only rule for msr, the /dev/cpu become 777, the /dev/cpu/0/ is
>>> 755 as it should be.
>>>
>>> Busybox mdev 1.20.2
>>
>> A bit more debuging, the rule:
>>
>>     msr0 root:root 600 =cpu/msr0
>>
>> Does work, but if I add / at the end, like:
>>
>>     msr0 root:root 600 =cpu/msr0/msr
>>
>> it does alter /dev/cpu permissions.
>
> Poor and wrong by design but gets the job done.
>
>  From a5f05ba5c03cd8b6d1e384b25a28013619329b48 Mon Sep 17 00:00:00 2001
> From: Piotr Karbowski <piotr.karbowski at gmail.com>
> Date: Fri, 21 Dec 2012 15:54:07 +0100
> Subject: [PATCH] umask(077) as a workaround for security vuln.
>
> Mdev seems to alter permissions of created dirs. Example:
> microcode        root:root 600 =cpu/
> cpu([0-9]+)        root:root 600 =cpu/%1/cpuid
> msr([0-9]+)        root:root 600 =cpu/%1/msr
>
> will make /dev/cpu a world-writtable dir.
> ---
>   util-linux/mdev.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/util-linux/mdev.c b/util-linux/mdev.c
> index 79871d3..b85e2d7 100644
> --- a/util-linux/mdev.c
> +++ b/util-linux/mdev.c
> @@ -823,7 +823,7 @@ int mdev_main(int argc UNUSED_PARAM, char **argv)
>       bb_sanitize_stdio();
>
>       /* Force the configuration file settings exactly */
> -    umask(0);
> +    umask(077);
>
>       xchdir("/dev");
>

*friendly bump*

The security flow in busybox is kind of not cool, if anyone of you can 
acctualy look into it would be good.

-- Piotr.