Local security threat + patch to fix

[Stanb-users]

The wrapper program could possibly be used by local
users to gain access to the mailman group and thus
compromise sensative information accessable by group
mailman. The offending code was introduced I think
in 2.0 beta3 and is still there in current CVS.

The problem is in the fatal() function, it takes user
input and fprintf's it without formatting it first.
Because wrapper is sgid and with permission setup the
way they are (all regular can access /home/mailman/mail
and wrapper because the directories and the program are
are set mode 2755. Anyway, here's the problem:

[root@king mailman-2.0beta3]# l /home/mailman/mail/wrapper
-rwxr-sr-x 1 root mailman 36562 Jul 27 23:46
/home/mailman/mail/wrapper
[root@king mailman-2.0beta3]# /home/mailman/mail/wrapper
Usage: /home/mailman/mail/wrapper program [args...]
[root@king mailman-2.0beta3]# /home/mailman/mail/wrapper dog
Illegal command: dog[root@king mailman-2.0beta3]#
[root@king mailman-2.0beta3]# /home/mailman/mail/wrapper %s%s
Illegal command: Illegal command: %s%sIýÿ¿œ=@$üÿ¿øûÿ¿€>@[root@king
mailman-2.0beta3]#
[root@king mailman-2.0beta3]# /home/mailman/mail/wrapper %U
Illegal command: %U[root@king mailman-2.0beta3]# /home/mailman/mail/wrapper
%u
Illegal command: 3221223312[root@king mailman-2.0beta3]#
/home/mailman/mail/wrapper %p
Illegal command: 0xbffff790[root@king mailman-2.0beta3]#
/home/mailman/mail/wrapper %s%s
Illegal command: Illegal command: %s%sIýÿ¿œ=@$üÿ¿øûÿ¿€>@[root@king
mailman-2.0beta3]#
[root@king mailman-2.0beta3]# /home/mailman/mail/wrapper %s%s%s
Segmentation fault

I know it looks badly placed but the error message doesn't
insert a newline so it gets bunched up. Anyway the
problem is quite simple. Here is another way it could
done:
[root@king mailman-2.0beta3]# doexec /home/mailman/mail/wrapper
Usage: (null) program [args...]
[root@king mailman-2.0beta3]# doexec /home/mailman/mail/wrapper %s
Usage: Usage: %s program [args...]
 program [args...]
[root@king mailman-2.0beta3]# doexec /home/mailman/mail/wrapper %s%s
Usage: Usage: %s%s program [args...]
Xýÿ¿œ=@Tüÿ¿(üÿ¿€>@ program [args...]
[root@king mailman-2.0beta3]# doexec /home/mailman/mail/wrapper %s%s%s
Segmentation fault

Anyway here's the patch to fix the prob in common.c
plus I fixed calls to fatal() which lacked \n at the
which are necessary since fatal() doesn't add one
on its own.

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;
 }

The patch was made against latest CVS release, but like
I said it affected Beta3 and Beta4 as well. Maybe you
should supply patch to users of that or release new
Beta5 to fix it or something if people really care :)

Good luck,

Stan Bubrouski (<email address hidden>)

[http://sourceforge.net/tracker/index.php?func=detail&aid=210401&group_id=103&atid=100103]

[Barry Warsaw (barry)]

I have applied this patch to 2.0b5, although I added the \n's in fatal()
and removed them elsewhere. Thanks!

BTW, the SourceForge patch manager is better for posting the patch text.