[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#655948: visudo: segfault on hurd-i386 due to calling unlink with null pointer



Package: sudo
Version: 1.8.3p1-3sjm1
Severity: normal
Tags: upstream patch
User: debian-hurd@lists.debian.org
Usertags: hurd

Hello,

After applying the patches in bugs #655883 and #655894, visudo
successfully edits /etc/sudoers, but then segfaults after moving the new
sudoers file into place. gdb backtrace:

#0  strrchr () at ../sysdeps/i386/strrchr.S:178
#1  0x010546cb in __hurd_directory_name_split (
    use_init_port=0x104e8b0 <_hurd_ports_use>, 
    get_dtable_port=0x106cb40 <__getdport>, lookup=0, file_name=0x0, 
    name=0x1024c08) at hurdlookup.c:176
#2  0x01054838 in __directory_name_split (directory_name=0x0, name=0x1024c08)
    at hurdlookup.c:256
#3  0x01133ecc in __unlink (name=0x0) at ../sysdeps/mach/hurd/unlink.c:34
#4  0x0804ab33 in install_sudoers (oldperms=<optimized out>, 
    sp=<optimized out>)
    at /home/steven/sudo/sudo-1.8.3p1/plugins/sudoers/visudo.c:602
#5  main (argc=<optimized out>, argv=<optimized out>)
    at /home/steven/sudo/sudo-1.8.3p1/plugins/sudoers/visudo.c:253

After discussion on the debian-hurd mailing list[1], it seems the problem
is that visudo.c skips editing files with sp->doedit set to 0, but then
doesn't skip installing these files back to their original locations,
resulting in a call to unlink with a null pointer (because there is no
corresponding temporary file, so sp->tpath is null). This means that when
editing /etc/sudoers with '#includedir /etc/sudoers.d' present, there is
always a segfault as visudo tries to unlink a null pointer while
processing /etc/sudoers.d/README.

This has gone unnoticed on other ports, because Linux and kFreeBSD's
unlink implementations simply return an error when passed a null pointer.

Attached is a fix provided on the debian-hurd mailing list by Pino
Toscano, which skips installing sudoers files that weren't edited. When
applied in combination with the patches in bugs #655883 and #655894,
visudo runs on hurd-i386 without any problems as far as I can tell.

Please let me know if you need any further information.

[1] http://lists.debian.org/debian-hurd/2012/01/msg00033.html

-- System Information:
Debian Release: wheezy/sid
  APT prefers unreleased
  APT policy: (500, 'unreleased'), (500, 'unstable')
Architecture: hurd-i386 (i686-AT386)

Kernel: GNU-Mach 1.3.99/Hurd-0.3
Locale: LANG=en_AU.UTF-8, LC_CTYPE=en_AU.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages sudo depends on:
ii  libc0.3         2.13-24
ii  libpam-modules  1.1.3-6
ii  libpam0g        1.1.3-6

sudo recommends no packages.

sudo suggests no packages.

-- Configuration Files:
/etc/sudoers [Errno 1073741837] Permission denied: u'/etc/sudoers'
/etc/sudoers.d/README [Errno 1073741837] Permission denied: u'/etc/sudoers.d/README'

-- no debconf information
Skip installing/unlinking files without doedit set
--- a/plugins/sudoers/visudo.c
+++ b/plugins/sudoers/visudo.c
@@ -247,6 +247,8 @@
 
     /* Install the sudoers temp files. */
     tq_foreach_fwd(&sudoerslist, sp) {
+	if (!sp->doedit)
+	    continue;
 	if (!sp->modified)
 	    (void) unlink(sp->tpath);
 	else

Reply to: