[src/trunk]: src/usr.bin/mail Fix various security related issues:

[christos <christos%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/8de047ceb600
branches:  trunk
changeset: 334945:8de047ceb600
user:      christos <christos%NetBSD.org@localhost>
date:      Tue Dec 16 19:30:24 2014 +0000

description:
Fix various security related issues:

    0001. Do not recognize paths, mail folders, and pipes in mail addresses
    by default.  That avoids a direct command injection with syntactically
    valid email addresses starting with |.

    Such addresses can be specified both on the command line, the mail
    headers (with -t) or in address lines copied over from previous
    while replying.

    This was assigned CVE-2014-7844 for some versions of BSD mailx.  It is
    documented behavior for Heirloom mailx, and was mentioned in an old
    technical report about BSD mailx (which does not usually make its way
    into operating system installations).  The patch switches off this
    processing and updates the documentation.

Added expandaddr option to explicitly enable this behavior.

    0002. When invoking sendmail, prevent option processing for email
    address arguments.  This prevents changing e.g. the Postfix
    configuration file in unexpected ways.  This behavior was documented for
    BSD mailx (sort of), but not for Heirloom mailx.  We did not assign a
    CVE to this because it is more of a missing feature, and code invoking
    mailx needs adjustment in the caller as well.

Fixed.

    0003. Make wordexp support mandatory.  (No functional change.)

Fixed (replaced explicit shell pipe implementation).

    0004. Prevent command execution in the expand function, which is IMHO
    unexpected.  (Not really required with patch 1, and there is still
    information disclosure/DoS potential if this expansion occurs.)  This is
    a historic vulnerability already fixed in the Debian package,
    retroactively assigned CVE-2004-2771:

Fixed (as part of the pipe replacement with wordexp).

diffstat:

 usr.bin/mail/cmd3.c   |   6 +-
 usr.bin/mail/extern.h |   6 +-
 usr.bin/mail/fio.c    |  94 ++++++++++++++++++++++++++------------------------
 usr.bin/mail/mail.1   |   8 +++-
 usr.bin/mail/names.c  |  15 +++++--
 usr.bin/mail/send.c   |   6 +-
 6 files changed, 75 insertions(+), 60 deletions(-)

diffs (truncated from 301 to 300 lines):

diff -r 53d63b89b2b8 -r 8de047ceb600 usr.bin/mail/cmd3.c
--- a/usr.bin/mail/cmd3.c       Tue Dec 16 17:00:17 2014 +0000
+++ b/usr.bin/mail/cmd3.c       Tue Dec 16 19:30:24 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cmd3.c,v 1.42 2012/04/29 23:50:22 christos Exp $       */
+/*     $NetBSD: cmd3.c,v 1.43 2014/12/16 19:30:24 christos Exp $       */
 
 /*
  * Copyright (c) 1980, 1993
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = "@(#)cmd3.c     8.2 (Berkeley) 4/20/95";
 #else
-__RCSID("$NetBSD: cmd3.c,v 1.42 2012/04/29 23:50:22 christos Exp $");
+__RCSID("$NetBSD: cmd3.c,v 1.43 2014/12/16 19:30:24 christos Exp $");
 #endif
 #endif /* not lint */
 
@@ -577,7 +577,7 @@
        if (hdr.h_to == NULL)
                return 1;
 
-       smargs = unpack(hdr.h_to);
+       smargs = unpack(NULL, hdr.h_to);
        for (ip = msgvec; *ip; ip++) {
                int e;
                if ((e = bounce_one(*ip, smargs, hdr.h_to)) != 0)
diff -r 53d63b89b2b8 -r 8de047ceb600 usr.bin/mail/extern.h
--- a/usr.bin/mail/extern.h     Tue Dec 16 17:00:17 2014 +0000
+++ b/usr.bin/mail/extern.h     Tue Dec 16 19:30:24 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: extern.h,v 1.32 2012/02/28 22:30:44 joerg Exp $        */
+/*     $NetBSD: extern.h,v 1.33 2014/12/16 19:30:24 christos Exp $     */
 
 /*-
  * Copyright (c) 1992, 1993
@@ -29,7 +29,7 @@
  * SUCH DAMAGE.
  *
  *     @(#)extern.h    8.2 (Berkeley) 4/20/95
- *     $NetBSD: extern.h,v 1.32 2012/02/28 22:30:44 joerg Exp $
+ *     $NetBSD: extern.h,v 1.33 2014/12/16 19:30:24 christos Exp $
  */
 
 #ifndef __EXTERN_H__
@@ -224,7 +224,7 @@
 struct name * gexpand(struct name *, struct grouphead *, int, int);
 struct name * nalloc(char [], int);
 struct name * outof(struct name *, FILE *, struct header *);
-const char ** unpack(struct name *);
+const char ** unpack(struct name *, struct name *);
 struct name * usermap(struct name *);
 #if 0
 void   prettyprint(struct name *);     /* commented out? */
diff -r 53d63b89b2b8 -r 8de047ceb600 usr.bin/mail/fio.c
--- a/usr.bin/mail/fio.c        Tue Dec 16 17:00:17 2014 +0000
+++ b/usr.bin/mail/fio.c        Tue Dec 16 19:30:24 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: fio.c,v 1.40 2013/03/09 19:43:07 christos Exp $        */
+/*     $NetBSD: fio.c,v 1.41 2014/12/16 19:30:24 christos Exp $        */
 
 /*
  * Copyright (c) 1980, 1993
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = "@(#)fio.c      8.2 (Berkeley) 4/20/95";
 #else
-__RCSID("$NetBSD: fio.c,v 1.40 2013/03/09 19:43:07 christos Exp $");
+__RCSID("$NetBSD: fio.c,v 1.41 2014/12/16 19:30:24 christos Exp $");
 #endif
 #endif /* not lint */
 
@@ -42,6 +42,7 @@
 #include "extern.h"
 #include "thread.h"
 #include "sig.h"
+#include <wordexp.h>
 
 /*
  * Mail -- a mail program
@@ -424,13 +425,10 @@
 expand(const char *name)
 {
        char xname[PATHSIZE];
-       char cmdbuf[PATHSIZE];          /* also used for file names */
-       pid_t pid;
-       ssize_t l;
-       char *cp;
-       const char *shellcmd;
-       int pivec[2];
-       struct stat sbuf;
+       char cmdbuf[PATHSIZE];
+       int e;
+       wordexp_t we; 
+       sigset_t nset, oset;
 
        /*
         * The order of evaluation is "%" and "#" expand into constants.
@@ -466,47 +464,53 @@
        }
        if (strpbrk(name, "~{[*?$`'\"\\") == NULL)
                return name;
-       if (pipe(pivec) < 0) {
-               warn("pipe");
-               return name;
-       }
-       (void)snprintf(cmdbuf, sizeof(cmdbuf), "echo %s", name);
-       if ((shellcmd = value(ENAME_SHELL)) == NULL)
-               shellcmd = _PATH_CSHELL;
-       pid = start_command(shellcmd, NULL, -1, pivec[1], "-c", cmdbuf, NULL);
-       if (pid < 0) {
-               (void)close(pivec[0]);
-               (void)close(pivec[1]);
+
+       *xname = '\0';
+
+       sigemptyset(&nset);
+       sigaddset(&nset, SIGCHLD);
+       sigprocmask(SIG_BLOCK, &nset, &oset);
+       e = wordexp(name, &we, WRDE_NOCMD);
+       sigprocmask(SIG_SETMASK, &oset, NULL);
+
+       switch (e) {
+       case 0: /* OK */
+       case WRDE_NOSPACE:
+               warnx("Out of memory expanding `%s'", name);
                return NULL;
-       }
-       (void)close(pivec[1]);
-       l = read(pivec[0], xname, sizeof(xname));
-       (void)close(pivec[0]);
-       if (wait_child(pid) < 0 && WTERMSIG(wait_status) != SIGPIPE) {
-               warnx("Expansion `%s' failed [%x]", cmdbuf, wait_status);
+       case WRDE_BADVAL:
+       case WRDE_BADCHAR:
+       case WRDE_SYNTAX:
+               warnx("Syntax error expanding `%s'", name);
+               return NULL;
+       case WRDE_CMDSUB:
+               warnx("Command substitution not allowed expanding `%s'",
+                   name);
+               return NULL;
+       default:
+               warnx("Unknown expansion error %d expanding `%s'", e, name);
                return NULL;
        }
-       if (l < 0) {
-               warn("read");
-               return NULL;
+
+       switch (we.we_wordc) {
+       case 0:
+               warnx("No match for `%s'", name);
+               break;
+       case 1:
+               if (strlen(we.we_wordv[1]) >= PATHSIZE)
+                       warnx("Expansion too long for `%s'", name);
+               strlcpy(xname, we.we_wordv[1], PATHSIZE);
+               break;
+       default:
+               warnx("Ambiguous expansion for `%s'", name);
+               break;
        }
-       if (l == 0) {
-               warnx("No match for `%s'", name);
-               return NULL;
-       }
-       if (l == sizeof(xname)) {
-               warnx("Expansion buffer overflow for `%s'", name);
+
+       wordfree(&we);
+       if (!*xname)
                return NULL;
-       }
-       xname[l] = '\0';
-       for (cp = &xname[l-1]; *cp == '\n' && cp > xname; cp--)
-               continue;
-       cp[1] = '\0';
-       if (strchr(xname, ' ') && stat(xname, &sbuf) < 0) {
-               warnx("Ambiguous expansion for `%s'", name);
-               return NULL;
-       }
-       return savestr(xname);
+       else
+               return savestr(xname);
 }
 
 /*
diff -r 53d63b89b2b8 -r 8de047ceb600 usr.bin/mail/mail.1
--- a/usr.bin/mail/mail.1       Tue Dec 16 17:00:17 2014 +0000
+++ b/usr.bin/mail/mail.1       Tue Dec 16 19:30:24 2014 +0000
@@ -1,4 +1,4 @@
-.\"    $NetBSD: mail.1,v 1.60 2013/03/09 19:43:20 christos Exp $
+.\"    $NetBSD: mail.1,v 1.61 2014/12/16 19:30:24 christos Exp $
 .\"
 .\" Copyright (c) 1980, 1990, 1993
 .\"    The Regents of the University of California.  All rights reserved.
@@ -29,7 +29,7 @@
 .\"
 .\"    @(#)mail.1      8.8 (Berkeley) 4/28/95
 .\"
-.Dd March 9, 2013
+.Dd December 15, 2014
 .Dt MAIL 1
 .Os
 .Sh NAME
@@ -725,6 +725,10 @@
 .Ar mbox
 file, or his edit file in
 .Fl f .
+.It Ic expandaddr
+If unset (the default), recipient addresses must be names of local mailboxes
+or Internet mail addresses.
+If set, shell expansion of existing mailbox names will be performed.
 .It Ic expose
 Expose the thread structure so all messages appear in header listings.
 (See
diff -r 53d63b89b2b8 -r 8de047ceb600 usr.bin/mail/names.c
--- a/usr.bin/mail/names.c      Tue Dec 16 17:00:17 2014 +0000
+++ b/usr.bin/mail/names.c      Tue Dec 16 19:30:24 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: names.c,v 1.30 2012/10/21 01:11:23 christos Exp $      */
+/*     $NetBSD: names.c,v 1.31 2014/12/16 19:30:24 christos Exp $      */
 
 /*
  * Copyright (c) 1980, 1993
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = "@(#)names.c    8.1 (Berkeley) 6/6/93";
 #else
-__RCSID("$NetBSD: names.c,v 1.30 2012/10/21 01:11:23 christos Exp $");
+__RCSID("$NetBSD: names.c,v 1.31 2014/12/16 19:30:24 christos Exp $");
 #endif
 #endif /* not lint */
 
@@ -254,6 +254,9 @@
        int ispipe;
        char tempname[PATHSIZE];
 
+       if (value("expandaddr") == NULL)
+               return names;
+
        begin = names;
        np = names;
        (void)time(&now);
@@ -532,7 +535,7 @@
  * Return an error if the name list won't fit.
  */
 PUBLIC const char **
-unpack(struct name *np)
+unpack(struct name *smopts, struct name *np)
 {
        const char **ap, **begin;
        struct name *n;
@@ -547,7 +550,7 @@
         * the terminating 0 pointer.  Additional spots may be needed
         * to pass along -f to the host mailer.
         */
-       extra = 2;
+       extra = 3 * count(smopts);
        extra++;
        metoo = value(ENAME_METOO) != NULL;
        if (metoo)
@@ -563,6 +566,10 @@
                *ap++ = "-m";
        if (verbose)
                *ap++ = "-v";
+       for (/*EMPTY*/; smopts != NULL; smopts = smopts->n_flink)
+               if ((smopts->n_type & GDEL) == 0)
+                       *ap++ = smopts->n_name;
+       *ap++ = "--";
        for (/*EMPTY*/; n != NULL; n = n->n_flink)
                if ((n->n_type & GDEL) == 0)
                        *ap++ = n->n_name;
diff -r 53d63b89b2b8 -r 8de047ceb600 usr.bin/mail/send.c
--- a/usr.bin/mail/send.c       Tue Dec 16 17:00:17 2014 +0000
+++ b/usr.bin/mail/send.c       Tue Dec 16 19:30:24 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: send.c,v 1.37 2012/04/29 23:50:22 christos Exp $       */
+/*     $NetBSD: send.c,v 1.38 2014/12/16 19:30:24 christos Exp $       */
 
 /*
  * Copyright (c) 1980, 1993
@@ -34,7 +34,7 @@
 #if 0
 static char sccsid[] = "@(#)send.c     8.1 (Berkeley) 6/6/93";
 #else
-__RCSID("$NetBSD: send.c,v 1.37 2012/04/29 23:50:22 christos Exp $");
+__RCSID("$NetBSD: send.c,v 1.38 2014/12/16 19:30:24 christos Exp $");
 #endif
 #endif /* not lint */
 
@@ -761,7 +761,7 @@
                                goto out;
                        }
        }
-       namelist = unpack(cat(hp->h_smopts, to));
+       namelist = unpack(hp->h_smopts, to);
        mail2(mtf, namelist);
  out: