Ubuntu

mountall creates /dev/.udev/rules.d/root.rules with mode 0666

Reported by Alasdair MacGregor on 2010-06-09
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
mountall (Ubuntu)
High
Unassigned
Lucid
High
Unassigned
Maverick
High
Unassigned

Bug Description

Binary package hint: mountall

Description: Ubuntu 10.04 LTS
Release: 10.04

mountall:
  Installed: 2.15
  Candidate: 2.15
  Version table:
 *** 2.15 0
        500 http://us.archive.ubuntu.com/ubuntu/ lucid-updates/main Packages
        100 /var/lib/dpkg/status
     2.14 0
        500 http://us.archive.ubuntu.com/ubuntu/ lucid/main Packages

mountall creates /dev/.udev/rules.d/root.rules as world-writable:

-rw-rw-rw- 1 root root 70 2010-06-09 07:31 /dev/.udev/rules.d/root.rules

Kees Cook (kees) wrote :

Touching this file after initramfs boot doesn't seem to trigger a udev rules reload, so I think this file isn't actually being used by udev any more. That said, it should not be world-writable. Scott, can you confirm this can't be used to load arbitrary udev rules?

Changed in mountall (Ubuntu):
status: New → Confirmed
importance: Undecided → Medium
Download full text (8.3 KiB)

udev certainly reloads this rules file when touched:

quest scott# touch /dev/.udev/rules.d/root.rules
1283260924.542059 [3873] parse_file: reading '/lib/udev/rules.d/40-fuse-utils.rules' as rules file
1283260924.542170 [3873] parse_file: reading '/lib/udev/rules.d/40-gnupg.rules' as rules file
1283260924.542212 [3873] parse_file: reading '/lib/udev/rules.d/40-hplip.rules' as rules file
1283260924.542464 [3873] parse_file: reading '/lib/udev/rules.d/40-ia64.rules' as rules file
1283260924.542492 [3873] parse_file: reading '/lib/udev/rules.d/40-infiniband.rules' as rules file
1283260924.542530 [3873] parse_file: reading '/lib/udev/rules.d/40-isdn.rules' as rules file
1283260924.542574 [3873] parse_file: reading '/lib/udev/rules.d/40-kino.rules' as rules file
1283260924.542605 [3873] parse_file: reading '/lib/udev/rules.d/40-libgphoto2-2.rules' as rules file
1283260924.548819 [3873] parse_file: reading '/lib/udev/rules.d/40-libpisock9.rules' as rules file
1283260924.548974 [3873] parse_file: reading '/lib/udev/rules.d/40-libsane.rules' as rules file
1283260924.550321 [3873] parse_file: reading '/lib/udev/rules.d/40-pilot-links.rules' as rules file
1283260924.550348 [3873] parse_file: reading '/lib/udev/rules.d/40-ppc.rules' as rules file
1283260924.550392 [3873] parse_file: reading '/lib/udev/rules.d/40-usb-media-players.rules' as rules file
1283260924.550850 [3873] parse_file: reading '/lib/udev/rules.d/40-xserver-xorg-video-intel.rules' as rules file
1283260924.550878 [3873] parse_file: reading '/lib/udev/rules.d/40-zaptel.rules' as rules file
1283260924.550914 [3873] parse_file: reading '/lib/udev/rules.d/45-fuse.rules' as rules file
1283260924.550939 [3873] parse_file: reading '/lib/udev/rules.d/45-libmtp8.rules' as rules file
1283260924.553885 [3873] parse_file: reading '/lib/udev/rules.d/50-firmware.rules' as rules file
1283260924.553914 [3873] parse_file: reading '/lib/udev/rules.d/50-udev-default.rules' as rules file
1283260924.554211 [3873] parse_file: reading '/lib/udev/rules.d/55-dm.rules' as rules file
1283260924.554286 [3873] parse_file: reading '/lib/udev/rules.d/56-hpmud_support.rules' as rules file
1283260924.554321 [3873] parse_file: reading '/lib/udev/rules.d/60-cdrom_id.rules' as rules file
1283260924.554349 [3873] parse_file: reading '/lib/udev/rules.d/60-floppy.rules' as rules file
1283260924.554378 [3873] parse_file: reading '/lib/udev/rules.d/60-persistent-alsa.rules' as rules file
1283260924.554425 [3873] parse_file: reading '/lib/udev/rules.d/60-persistent-input.rules' as rules file
1283260924.554514 [3873] parse_file: reading '/lib/udev/rules.d/60-persistent-serial.rules' as rules file
1283260924.554572 [3873] parse_file: reading '/lib/udev/rules.d/60-persistent-storage-dm.rules' as rules file
1283260924.554623 [3873] parse_file: reading '/lib/udev/rules.d/60-persistent-storage-tape.rules' as rules file
1283260924.554697 [3873] parse_file: reading '/lib/udev/rules.d/60-persistent-storage.rules' as rules file
1283260924.554868 [3873] parse_file: reading '/lib/udev/rules.d/60-persistent-v4l.rules' as rules file
1283260924.554918 [3873] parse_file: reading '/lib/udev/rules.d/61-gnome-bluetooth-rfkill.rules' as rules file
...

Read more...

I seem to have forgotten my usual umask() diligence around this block (it's there in every other, e.g. just above it) - I guess because this was rushed at the last minute.

The code should almost certainly read:

+ mask = umask (0022);
                        mkdir ("/dev/.udev", 0755);
                        mkdir ("/dev/.udev/rules.d", 0755);
                        rules = fopen ("/dev/.udev/rules.d/root.rules", "w");
                        if (rules) {
                                fprintf (rules, ("SUBSYSTEM==\"block\", "
                                                 "ENV{MAJOR}==\"%d\", "
                                                 "ENV{MINOR}==\"%d\", "
                                                 "SYMLINK+=\"root\"\n"),
                                         major (root->mounted_dev),
                                         minor (root->mounted_dev));
                                fclose (rules);
                        }
+ umask (mask);

Kees Cook (kees) wrote :

I can't reproduce the rule reloading. What steps did you take to show that? If I strace all the udevd processes, none of them appear to wake up when I touch the file.

Kees Cook (kees) wrote :

Actually... if I _restart_ udev, it will suddenly start watching this location. Which would seem to indicate there is a race condition here that may be saving most people from this vulnerability.

Kees Cook (kees) wrote :

CVE-2010-2961

Kees Cook (kees) wrote :

I'd like to publish fixes for this ASAP, which looks like after Maverick beta unfreezes the archive on Thursday. Code fix plus postinst that fixes the permissions on root.rules would seem to be all that is needed, correct?

Changed in mountall (Ubuntu Lucid):
status: New → Triaged
Changed in mountall (Ubuntu Maverick):
status: Confirmed → Triaged
importance: Medium → High
Changed in mountall (Ubuntu Lucid):
importance: Undecided → High
Changed in mountall (Ubuntu Maverick):
milestone: none → ubuntu-10.10-beta
milestone: ubuntu-10.10-beta → none
Kees Cook (kees) wrote :

Actually, we need to rebuild the /dev/.udev/rules.d/root.rules file in the postinst so that anything that may have gotten leaked into it is removed before allowing udev to reload:

umask 0022
rm -f /dev/.udev/rules.d/root.rules
if [ -l /dev/root ]; then
    printf 'SUBSYSTEM=="block", ENV{MAJOR}=="%d", ENV{MINOR}=="%d", SYMLINK+="root"\n' $(/usr/bin/stat -L -c '0x%t 0x%T' /dev/root) > /dev/.udev/rules.d/root.rules
fi

Kees Cook (kees) wrote :

s/-l/-L/

Kees Cook (kees) wrote :
tags: added: patch
Kees Cook (kees) wrote :
Kees Cook (kees) on 2010-09-08
Changed in mountall (Ubuntu Maverick):
status: Triaged → Fix Committed
Changed in mountall (Ubuntu Lucid):
status: Triaged → Fix Committed
visibility: private → public
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package mountall - 2.17

---------------
mountall (2.17) maverick; urgency=low

  * SECURITY UPDATE: do not leave writable udev rules file around.
    - src/mountall.c: set umask correctly (LP: #591807).
    - debian/preinst: remove boot-time udev rules file.
    - CVE-2010-2961
 -- Kees Cook <email address hidden> Wed, 01 Sep 2010 15:20:14 -0700

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package mountall - 2.15.2

---------------
mountall (2.15.2) lucid-security; urgency=low

  * SECURITY UPDATE: do not leave writable udev rules file around.
    - src/mountall.c: set umask correctly (LP: #591807).
    - debian/preinst: remove boot-time udev rules file.
    - CVE-2010-2961
 -- Kees Cook <email address hidden> Wed, 01 Sep 2010 15:30:44 -0700

Changed in mountall (Ubuntu Lucid):
status: Fix Committed → Fix Released
Changed in mountall (Ubuntu Maverick):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers