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

Denys Vlasenko vda.linux at googlemail.com
Mon Jan 21 00:23:29 UTC 2013


On Sunday 06 January 2013 18:46, Piotr Karbowski wrote:
> 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.

Fixed in git:

commit 4609f477c7e043a4f6147dfe6e86b775da2ef784
Author: Denys Vlasenko <vda.linux at googlemail.com>
Date:   Mon Jan 21 01:22:12 2013 +0100

    mdev: fix mode of dir1 in =dir1/dir2/file rule



Thanks!

-- 
vda


More information about the busybox mailing list