Date:         Tue, 1 Aug 2000 13:14:20 -0400
From: Stan Bubrouski <secnet@CROSSWINDS.NET>
Subject:      Advisory: mailman local compromise
To: BUGTRAQ@SECURITYFOCUS.COM

Author            : Stan Bubrouski
Date              : August 1, 2000
Package           : mailman
Versions affected : 2.0beta3 (released: 2000-Jun-28 23:25)
                    2.0beta4 (released: 2000-Jul-06 21:27)
Severity          : 

Most directories in a mailman install are mode 2755 as are most of the
binaries and scripts.  Many configurations are 664 allowing a local user to
change list ocnfigurations and even read the adm.pw passwd file.
Additionally besides being able to read public and private data along with
passwds, a malicious user could replace binaries and scripts in a mailman
installation because they are writable by group mailman.


Problem            : 

The mailman package comes with a sgid program named wrapper.  This program
contains a function named fatal() which is used to display error messages.
Unfortunately it fails to send the correct amount of arguments to the
fprintf(3) function allowing users to add formatting which could be used to
insert and execute code under group mailman.  fatal() is called when
invalid arguments are provided and in such a case, the invalid arguments
are sent to fprintf without being formatted, the same goes for argv[0].

Example:

[user@king user]$ ls -al /usr/share/mailman/mail/wrapper
-rwxr-sr-x    1 mailman  mailman     36290 Jul  1 06:48 
/usr/share/mailman/mail/wrapper
[user@king user]$ cd /usr/share/mailman/mail
[user@king mail]$ ls -al
total 39
drwxrwsr-x    2 mailman  mailman      1024 Jul 12 19:29 .
drwxrwsr-x   16 mailman  mailman      1024 Jul 27 20:13 ..
-rwxr-sr-x    1 mailman  mailman     36290 Jul  1 06:48 wrapper
[user@king mail]$ ./wrapper
Usage: ./wrapper program [args...]
[user@king mail]$ ./wrapper %s
Illegal command: Illegal command: %s[user@king mail]$ ./wrapper %s%s
Illegal command: Illegal command: %s%s•ýÿ¿œ=@„üÿ¿Xüÿ¿€>@[user@king 
mail]$ ./wrapper %s%s%s
Segmentation fault
[user@king mail]$ ./wrapper %s%u%p
Illegal command: Illegal command: %s%u%p32212244600x656c6c49[user@king mail]$
[user@king mail]$ doexec ./wrapper %s
Usage: Usage: %s program [args...]
  program [args...]
[user@king mail]$ doexec ./wrapper %s%s
Usage: Usage: %s%s program [args...]
“ýÿ¿œ=@„üÿ¿Xüÿ¿€>@
 program [args...]
[user@king mail]$ doexec ./wrapper %s%p
Usage: Usage: %s%p program [args...]
0xbffffc0c program [args...]
[user@king mail]$ doexec ./wrapper %s%S%u
Usage: Usage: %s%S%u program [args...]
[user@king mail]$ doexec ./wrapper %s%s
Usage: Usage: %s%s program [args...]
“ýÿ¿œ=@„üÿ¿Xüÿ¿€>@
 program [args...]
[user@king mail]$ doexec ./wrapper %s%s%s
Segmentation fault
[user@king mail]$

Patch:
diff -u -r ./cgi-wrapper.c.orig ./cgi-wrapper.c
--- ./cgi-wrapper.c.orig        Tue Mar 21 01:26:41 2000
+++ ./cgi-wrapper.c     Fri Jul 28 00:17:42 2000
@@ -53,7 +53,7 @@
         fake_argv[2] = script;

         status = run_script("driver", 3, fake_argv, env);
-       fatal(logident, status, "%s", strerror(errno));
+       fatal(logident, status, "%s\n", strerror(errno));
         return status;
  }

diff -u -r common.c.orig ./common.c
--- ./common.c.orig     Mon May 22 14:59:31 2000
+++ ./common.c  Thu Jul 27 23:58:12 2000
@@ -108,7 +108,7 @@
                 printf("</pre>\n");
         }
         else
-               fprintf(stderr, log_entry);
+               fprintf(stderr, "%s", log_entry);
  #endif /* HELPFUL */
         exit(exitcode);
  }
diff -u -r ./mail-wrapper.c.orig ./mail-wrapper.c
--- ./mail-wrapper.c.orig       Tue Mar 21 01:26:41 2000
+++ ./mail-wrapper.c    Fri Jul 28 00:16:34 2000
@@ -67,13 +67,13 @@

         if (!check_command(argv[1]))
                 fatal(logident, MAIL_ILLEGAL_COMMAND,
-                     "Illegal command: %s", argv[1]);
+                     "Illegal command: %s\n", argv[1]);

         check_caller(logident, parentgid);

         /* If we got here, everything must be OK */
         status = run_script(argv[1], argc, argv, env);
-       fatal(logident, status, "%s", strerror(errno));
+       fatal(logident, status, "%s\n", strerror(errno));
         return status;
  }

Patch info         : 

The patch fixes fatal() and also adds newlines to some fatal() calls
because fatal() does not tack them on and as you can see in the example
above, the lack of newlines in some calls make errors harder to read.  I
made the patch using the latest CVS tree but it should apply to beta3 and
beta4 releases as well.