ports/112754: [ports]: VERY SERIOUS security bug in sysutils/eject

[Ighighi]

>Number:         112754
>Category:       ports
>Synopsis:       [ports]: VERY SERIOUS security bug in sysutils/eject
>Confidential:   no
>Severity:       critical
>Priority:       high
>Responsible:    freebsd-ports-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Fri May 18 04:20:01 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Ighighi
>Release:        6.2-STABLE
>Organization:
>Environment:
FreeBSD orion 6.2-STABLE FreeBSD 6.2-STABLE #0: Wed May 16 13:47:58 VET 2007     root at orion:/usr/obj/usr/src/sys/CUSTOM  i386
>Description:
/usr/local/sbin/eject is installed setuid to root by default and the -f (force) option can be used by ANY user to unmount virtually any partition in /dev that he/she didn't mount.

NOTES:

+ 1.- The most sensible way to do this would be to call /sbin/umount directly after dropping privileges or import its functionality and/or check both the MNT_USER flag in struct statfs's f_flags and f_owner and the real user ID.  All of this is too much overkill for a program that essentially performs the same function that commands such as cdcontrol(8) and camcontrol(8) already do for ATAPI & ATAPI-CAM/SCSI CD-ROMS respectively, without root privileges and better... with /etc/devfs.conf providing enough flexibility and being well documented in the handbook.

+ 2.- This program seems to me to be to have been coded with absolutely no security in mind at all as if the "BINMODE= 4555" line in the Makefile was added later.  It doesn't check the return value of some functions (the call to strdup() for example), no use of "const char *", uninitialized integers (e.g, "sts"), etc.  Check also and line 145 of eject.c .  The output of "gcc -Wall" (after the system's patches have been applied) is:
$ gcc -Wall eject.c
eject.c:63: warning: return type defaults to `int'
eject.c: In function `check_device':
eject.c:145: warning: char format, pointer arg (arg 3)
eject.c: In function `unmount_fs':
eject.c:192: warning: left-hand operand of comma expression has no effect

+ 3.- It happens independently of the value of vfs.usermount.  (This is expected behavior anyway).

+ 4.- A shell script of mine is available to anyone upon request that extends the functionality of eject(1) that adds a -t option to close the CD tray that was never implemented by judging at CVS logs.  It could be extended to use camcontrol so "camcontrol eject -n cd -u 0 -v" is analogous to "cdcontrol -f /dev/acd0 eject" to manage /dev/cdX devices.

+ 5.- A few months back, when the ext2fs wasn't as stable as it is today, I could have panicked my system with the Poc above.

+ 6. I just hope no problem arises from this advisory so no karma will ever reach me. =)
>How-To-Repeat:
$ /bin/ls -lo /usr/local/sbin/eject
-r-sr-xr-x  1 root  wheel  - 5872 Jun 26  2006 /usr/local/sbin/eject

$ id
uid=501(ighighi) gid=501(ighighi) groups=501(ighighi),69(network)

$ /sbin/mount | grep ad3s7
/dev/ad3s7 on /mnt/linux/var (ext2fs, local, noatime, nosuid, read-only)

$ /usr/local/sbin/eject -vf /dev/ad3s7
eject: using device
eject: /dev/ad3s7 mounted on /mnt/linux/var
eject: force unmounting /mnt/linux/var
eject: ejecting media from /dev/ad3s7
eject: Inappropriate ioctl for device

$ /sbin/mount -v | grep ad3s7

>Fix:
Attached patch available (copy it to /usr/ports/sysutils/eject/files/ as patch-Makefile and reinstall) or simply run:
chmod -s /usr/local/sbin/eject


Patch attached with submission follows:

#
# (c) 2007 by Ighighi
#
# To enable this patch copy it to /usr/ports/sysutils/eject/files
# and reinstall
#

--- Makefile.orig	Tue Jan  4 10:32:55 2000
+++ Makefile	Thu May 17 05:25:57 2007
@@ -5,7 +5,7 @@
 PROG= eject
 
 BINOWN= root
-BINMODE= 4555
+BINMODE= 555
 BINDIR= ${PREFIX}/sbin
 
 MANDIR= ${PREFIX}/man/man

>Release-Note:
>Audit-Trail:
>Unformatted: