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

Bug #591807 reported by Alasdair MacGregor
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
mountall (Ubuntu)
Fix Released
High
Unassigned
Lucid
Fix Released
High
Unassigned
Maverick
Fix Released
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

Tags: patch
Revision history for this message
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
Revision history for this message
Scott James Remnant (Canonical) (canonical-scott) wrote :
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...

Revision history for this message
Scott James Remnant (Canonical) (canonical-scott) wrote :

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);

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
Kees Cook (kees) wrote :

CVE-2010-2961

Revision history for this message
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
Revision history for this message
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

Revision history for this message
Kees Cook (kees) wrote :

s/-l/-L/

Revision history for this message
Kees Cook (kees) wrote :
tags: added: patch
Revision history for this message
Kees Cook (kees) wrote :
Kees Cook (kees)
Changed in mountall (Ubuntu Maverick):
status: Triaged → Fix Committed
Changed in mountall (Ubuntu Lucid):
status: Triaged → Fix Committed
visibility: private → public
Revision history for this message
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

Revision history for this message
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  
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.