Package: nvi
Version: 1.79-21
Priority: grave
Tags: security patch woody sid
Justification: local DoS

(Note: The bugs I talk about in this report have been present in Debian's
nvi for ages. Actually, OpenBSD provides an alternate 'recover'
implementation (attached) written in Perl that fixes most of this stuff in
probably a better way since it includes additional cheks than my patch to
the init.d file (also attached).)

Doing a security audit review of /tmp usage I've found that the
/etc/init.d/nviboot (it is also provided with other names in woody in the
nvi-m17n source package) is not coded in order to prevent some situations
that could be forced by local users to generate unexpected events. The fact
that this script runs as root on startup makes the bugs serious even if a
direct attack is not self evident.

The script has the following issues:

- Will run even if the binary and directory it uses are not available
- Will interpret any file even if it has been created by non-interactive 
users that do not normally create vi.recover files
- Will follow symlinks
- Will work with directories, even if this generates errors later on (when 
trying to read from them)
- PATH is unset

All of these are fixed in the attached patch.

If you think that the patch is not worth it, consider the following 
hypotheses:

- A local user symlinks "recover.test" to /dev/zero: will cause a DoS when
the system whenever the system starts up. This is a local attack.

- A remote user compromises a service server in which mail use is
restricted to a given group (this is non-standard configuration, can be 
implemented through capabitities or just chmoding 660 /usr/sbin/sendmail 
or whatever it points to) and creates a "recover.test" file with:
.................................................
To: hacker
From: root
X-vi-recover-path: /etc/passwd

System rebooted!

.
.................................................

He will get a mail whenever the system is rebooted. This can also be used 
by local users to play tricks on other users ("I just got a mail from 
root saying I should change my password to '111111'" :-)

- A bug is found in exim's implementation of sendmail -t (buffer overflow
when parsing headers?). When 'sendmail -t' is run with a rogue
'recover.hackme' file a local user can attempt a privilege escalation when
the server is rebooted since 'sendmail -t' will be run on whatever is in 
/var/tmp/vi.recover as root.

- A local user creates a "recover.test" directory: error on startup


The patch:

- will delete any symlink files found in /var/tmp/vi.recover

- will refuse to work with directories that resemble recover files

- will call the sendmail binary with lower (nobody) privileges to prevent a 
bug in sendmail from becoming a privilege escalation possibility.

- will delete any files found if they don't below to a "Joe user". It has 
the cavea that root nvi recover sessions will also be removed (probably 
one of the few admin user which will use nvi often). This behaviour can be 
switched on or off.

- will set a proper PATH

Please include this patch or a similar one in the next release.
BTW, an alternative for the "won't send mails of admin users" it to not 
honor the content of the recover file and mail to the owner of the file 
(instead of to whomever the file says to in its 'To:' 'Resent-To', 'Bcc:', 
'Cc:', 'Resent-Cc:' and 'Resent-Bcc:'

Thanks

Javier

--------------------------------------------------------------------------

--- init.orig   2005-03-01 00:18:16.000000000 +0100
+++ init        2005-03-01 00:49:38.000000000 +0100
@@ -3,8 +3,20 @@
 #
 # Script to recover nvi edit sessions.
 #
+PATH=/sbin:/usr/sbin:/bin:/usr/bin
 RECDIR=/var/tmp/vi.recover
 SENDMAIL=/usr/sbin/sendmail
+SYSTEMUIDS="no"
+
+[ ! -x "$SENDMAIL" ] && exit 0
+[ ! -d "$RECDIR" ] && exit 0
+
+FIRST_UID=""
+if [ -r /etc/adduser.conf ] ; then
+       FIRST_UID=`grep ^FIRST_UID /etc/adduser.conf | awk -F = '{print $2}'`
+fi
+# Sane default
+[ -z "$FIRST_UID" ] && FIRST_UID=1000
 
 case "$1" in
   start)
@@ -18,8 +30,15 @@
             # would only happen if some loser is playing games with
             # embedded spaces in vi recovery file names
             i=$RECDIR/${i#$RECDIR/}
+               # Nvi editor backup files should not be symlinks.
+               # Delete them
+               if test -L $i ; then
+                       rm $i
+                       continue
+               fi
+
                # Only test files that are readable.
-               if test ! -r $i; then
+               if test ! -r $i -o test ! -f $i; then
                        continue
                fi
 
@@ -27,7 +46,17 @@
                # execute bit set or are zero length.  Delete them.
                if test -x $i -o ! -s $i; then
                        rm $i
+                       continue
                fi
+
+
+               # Files that belong to administrative users are
+               # discarded
+               if [ "$SYSTEMUIDS" = "no" ] && \
+                  [ "`/usr/bin/stat -c %u $i`" -lt "$FIRST_UID" ]; then
+                  rm $i
+               fi
+
        done
     fi
 
@@ -40,17 +69,30 @@
             # would only happen if some loser is playing games with
             # embedded spaces in vi recovery file names
             i=$RECDIR/${i#$RECDIR/}
+               # Discard symlinks
+               if test -L $i ; then
+                       rm $i
+                       continue
+               fi
                # Only test files that are readable.
-               if test ! -r $i; then
+               if test ! -r $i -o ! -f $i; then
                        continue
                fi
 
+               # Files that belong to administrative users are
+               # discarded
+               if [ "$SYSTEMUIDS" = "no" ] && \
+                  [ "`/usr/bin/stat -c %u $i`" -lt "$FIRST_UID" ]; then
+                  rm $i
+                  continue
+               fi
+
                # Delete any recovery files that are zero length, corrupted,
                # or that have no corresponding backup file.  Else send mail
                # to the user.
                recfile=`awk '/^X-vi-recover-path:/{print $2}' < $i`
                if test -n "$recfile" -a -s "$recfile"; then
-                       ($SENDMAIL -t < $i &) </dev/null >/dev/null 2>&0
+                       (su - nobody -c $SENDMAIL -t < $i &) </dev/null 
>/dev/null 2>&0
                else
                        rm $i
                fi

--------------------------------------------------------------------------

#!/usr/bin/perl -w
#
# $OpenBSD: recover,v 1.9 2001/11/06 23:31:08 millert Exp $
#
# Script to (safely) recover nvi edit sessions.
#

use Fcntl;
require 'sys/syscall.ph';

$recoverdir = $ARGV[0] || "/var/tmp/vi.recover";
$sendmail = "/usr/sbin/sendmail";

die "Sorry, $0 must be run as root\n" if $>;

# Make the recovery dir if it does not already exist.
if (!sysopen(DIR, $recoverdir, O_RDONLY|O_NOFOLLOW) || !stat(DIR)) {
        die "Warning! $recoverdir is a symbolic link! (ignoring)\n"
            if -l $recoverdir;
        mkdir($recoverdir, 01777) || die "Unable to create $recoverdir: $!\n";
        chmod(01777, $recoverdir);
        exit(0);
}

#
# Sanity check the vi recovery dir
# Perl doesn't support fchdir, fchmod, or fchown so we call
# fchdir(2) via the syscall interface and then just modify ".".
#
die "Warning! $recoverdir is not a directory! (ignoring)\n"
    unless -d _;
die "$0: can't chdir to $recoverdir: $!\n"
    unless syscall(&SYS_fchdir, fileno(DIR)) == 0;
if (! -O _) {
        warn "Warning! $recoverdir is not owned by root! (fixing)\n";
        chown(0, 0, ".");
}
if (((stat(_))[2] & 07777) != 01777) {
        warn "Warning! $recoverdir is not mode 01777! (fixing)\n";
        chmod(01777, ".");
}

# Check editor backup files.
opendir(RECDIR, ".") || die "$0: can't open $recoverdir: $!\n";
foreach $file (readdir(RECDIR)) {
        next unless $file =~ /^vi\./;

        #
        # Unmodified vi editor backup files either have the
        # execute bit set or are zero length.  Delete them.
        # Anything that is not a normal file gets deleted too.
        #
        lstat($file) || die "$0: can't stat $file: $!\n";
        if (-x _ || ! -s _ || ! -f _) {
                unlink($file) unless -d _;
        }
}

#
# It is possible to get incomplete recovery files if the editor crashes
# at the right time.
#
rewinddir(RECDIR);
foreach $file (readdir(RECDIR)) {
        next unless $file =~ /^recover\./;

        if (!sysopen(RECFILE, $file, O_RDONLY|O_NOFOLLOW)) {
            warn "$0: can't open $file: $!\n";
            next;
        }

        #
        # Delete anything that is not a regular file as that is either
        # filesystem corruption from fsck or an exploit attempt.
        #
        if (!stat(RECFILE)) {
                warn "$0: can't stat $file: $!\n";
                close(RECFILE);
                next;
        }
        $owner = (stat(_))[4];
        if (! -f _ || ! -s _) {
                unlink($file) unless -d _;
                close(RECFILE);
                next;
        }

        #
        # Slurp in the recover.* file and search for X-vi-recover-path
        # (which should point to an existing vi.* file).
        #
        @recfile = <RECFILE>;
        close(RECFILE);

        #
        # Delete any recovery files that have no (or more than one)
        # corresponding backup file.
        #
        @backups = grep(m#^X-vi-recover-path:\s*\Q$recoverdir\E/+#, @recfile);
        if (@backups != 1) {
                unlink($file);
                next;
        }

        #
        # Make a copy of the backup file path.
        # We must not modify @backups directly since it contains
        # references to data in @recfile which we pipe to sendmail.
        #
        $backups[0] =~ m#^X-vi-recover-path:\s*\Q$recoverdir\E/+(.*)[\r\n]*$#;
        $backup = $1;

        #
        # If backup file is not rooted in the recover dir, ignore it.
        # If backup file owner doesn't match recovery file owner, ignore it.
        # If backup file is zero length or not a regular file, remove it.
        # Else send mail to the user.
        #
        if ($backup =~ m#/# || !lstat($backup)) {
                unlink($file);
        } elsif ($owner != 0 && (stat(_))[4] != $owner) {
                unlink($file);
        } elsif (! -f _ || ! -s _) {
                unlink($file, $backup);
        } else {
                open(SENDMAIL, "|$sendmail -t") ||
                    die "$0: can't run $sendmail -t: $!\n";
                print SENDMAIL @recfile;
                close(SENDMAIL);
        }
}
closedir(RECDIR);
close(DIR);

exit(0);




----- End forwarded message -----

-- 

Attachment: signature.asc
Description: Digital signature

Reply via email to