Hi all,
I hope this message gets through since I'm not subscribed
to the list...
I'm attaching a patch that hopefully addresses the locally
exploitable security problems discussed recently on bugtraq
and here.
This patch greatly restricts what applications you can call through
wrapper, and with what arguments.
1. It puts majordomo and resend in a subdirectory
called sbin and restricts wrapper to executing
those.
This keeps Joe user from executing commands such as
$LIB/wrapper config-test /tmp/myjordomo.cf
2. The only args it allows right now is -h for
hostname, and -l for listname. These names
must consist entirely of alphanumerics, plus
a few specials (like -_+.).
For the resend command, you're allowed to specify
additional arguments (the outgoing alias(es)),
but special checks are made that these don't start
with - or @.
This keeps Joe user from sneaking in config files
through various flags. It also keeps him from
using list names such as ../../../../tmp/foo
to create majordomo owned files anywhere he wants.
3. resend refuses to serve lists that don't have
a config file. This keeps Joe user from creating
arbitrary list files.
4. It fixes a temp race condition in the creation of
{majordomo.resend}.debug simply by not creating
these files unless you've set $DEBUG = 1.
Now for a question to the authors of majordomo: we, Caldera, ship
majordomo as part of our distribution. Now the license seems to forbid
us to apply any patches. Is there a formal (and speedy:) process to
get the security fix appended below approved?
Olaf
--
Olaf Kirch | --- o --- Nous sommes du soleil we love when we play
okir@monad.swb.de | / | \ sol.dhoop.naytheet.ah kin.ir.samse.qurax
okir@caldera.de +-------------------- Why Not?! -----------------------
UNIX, n.: Spanish manufacturer of fire extinguishers.
diff -ur majordomo-1.94.4.orig/Makefile majordomo-1.94.4/Makefile
--- majordomo-1.94.4.orig/Makefile Wed Jan 5 10:22:44 2000
+++ majordomo-1.94.4/Makefile Wed Jan 5 11:58:39 2000
@@ -75,7 +75,8 @@
# For those stupid machines that try to use csh. Doh!
SHELL = /bin/sh
-WRAPPER_FLAGS = -DBIN=\"$(W_HOME)\" -DPATH=\"PATH=$(W_PATH)\" \
+WRAPPER_FLAGS = -DBIN=\"$(W_HOME)\" -DSBIN=\"$(W_HOME)/sbin\" \
+ -DPATH=\"PATH=$(W_PATH)\" \
-DHOME=\"HOME=$(W_HOME)\" -DSHELL=\"SHELL=$(W_SHELL)\" \
-DMAJORDOMO_CF=\"MAJORDOMO_CF=$(W_MAJORDOMO_CF)\" \
-DSETGROUP $(POSIX)
@@ -93,6 +94,7 @@
BIN = bounce-remind config_parse.pl majordomo majordomo.pl \
majordomo_version.pl request-answer resend \
shlock.pl config-test archive2.pl digest
+BINSBIN = majordomo resend
INSTALL_FLAGS = -O $(W_USER) -g $(W_GROUP)
@@ -145,6 +147,7 @@
install-scripts: config-scripts
$(INSTALL) -m $(HOME_MODE) $(INSTALL_FLAGS) . $(W_HOME)
$(INSTALL) -m $(EXEC_MODE) $(INSTALL_FLAGS) . $(W_HOME)/bin
+ $(INSTALL) -m $(EXEC_MODE) $(INSTALL_FLAGS) . $(W_HOME)/sbin
@echo "Copying tools to $(W_HOME)/bin"
@@ -167,6 +170,11 @@
@for file in $(TOOLS); do \
$(INSTALL) -m $(EXEC_MODE) $(INSTALL_FLAGS) \
$(TMP)/$$file $(W_HOME)/Tools/$$file; \
+ done
+
+ @echo "Linking $(BINSBIN) to $(W_HOME)/sbin"
+ @for file in $(BINSBIN); do \
+ ln -s ../$$file $(W_HOME)/sbin/$$file; \
done
@rm -rf $(TMP)
diff -ur majordomo-1.94.4.orig/config_parse.pl majordomo-1.94.4/config_parse.pl
--- majordomo-1.94.4.orig/config_parse.pl Wed Aug 27 16:41:32 1997
+++ majordomo-1.94.4/config_parse.pl Wed Jan 5 11:58:39 2000
@@ -1,3 +1,4 @@
+#!/usr/bin/perl
'di';
'ig00';
# A file to parse a majordomo mailing list config file
diff -ur majordomo-1.94.4.orig/majordomo majordomo-1.94.4/majordomo
--- majordomo-1.94.4.orig/majordomo Wed Aug 27 16:55:29 1997
+++ majordomo-1.94.4/majordomo Wed Jan 5 12:03:42 2000
@@ -45,12 +45,14 @@
}
require "$cf";
+$ENV{"MAJORDOMO_CF"} = $cf;
# Go to the home directory specified by the .cf file
chdir("$homedir") || die "chdir to $homedir failed, $!\n";
# If standard error is not attached to a terminal, redirect it to a file.
-if (! -t STDERR) {
+# Only do this when DEBUG is set in order to avoid /tmp races
+if (! -t STDERR && $DEBUG) {
close STDERR;
open (STDERR, ">>$TMPDIR/majordomo.debug");
}
diff -ur majordomo-1.94.4.orig/majordomo.pl majordomo-1.94.4/majordomo.pl
--- majordomo-1.94.4.orig/majordomo.pl Wed Aug 27 16:58:53 1997
+++ majordomo-1.94.4/majordomo.pl Wed Jan 5 11:58:39 2000
@@ -1,3 +1,4 @@
+#!/usr/bin/perl
# General subroutines for Majordomo
# $Source: /sources/cvsrepos/majordomo/majordomo.pl,v $
@@ -324,7 +325,7 @@
}
# These are package globals referenced by &setlogfile and &log
-$log_file = "/tmp/log.$$";
+$log_file = "$TMPDIR/majordomo.log.$$";
$log_host = "UNKNOWN";
$log_program = "UNKNOWN";
$log_session = "UNKNOWN";
diff -ur majordomo-1.94.4.orig/majordomo_version.pl majordomo-1.94.4/majordomo_version.pl
--- majordomo-1.94.4.orig/majordomo_version.pl Wed Aug 27 17:56:36 1997
+++ majordomo-1.94.4/majordomo_version.pl Wed Jan 5 11:58:39 2000
@@ -1,3 +1,4 @@
+#!/usr/bin/perl
# $Header: /sources/cvsrepos/majordomo/majordomo_version.pl,v 1.26 1997/08/27 15:56:36 cwilson Exp $
$majordomo_version = "1.94.4";
diff -ur majordomo-1.94.4.orig/resend majordomo-1.94.4/resend
--- majordomo-1.94.4.orig/resend Wed Jan 5 10:22:44 2000
+++ majordomo-1.94.4/resend Wed Jan 5 12:03:14 2000
@@ -127,8 +127,8 @@
# if we're running from a tty, just spit to stderr, else
# open up a temp file for the debug output.
-#
-if (! -t STDERR) {
+# Only do this when debugging is enabled in order to avoid races
+if (! -t STDERR && $DEBUG) {
close STDERR;
open (STDERR, ">>$TMPDIR/resend.debug");
}
@@ -145,6 +145,13 @@
# this doesn't have to be this way. It could slurp it
# from the alias it was invoked as...?
}
+
+# Resending to a non-existant list should fail rather than
+# create a list config for it (as main'get_config does)
+if (! -e "$listdir/$opt_l.config") {
+ die "Unknown list $opt_l\n";
+}
+
# A classic case of feeping creaturism. While there are possibly good reasons
# why all these things can be classified on the command line, there's
diff -ur majordomo-1.94.4.orig/shlock.pl majordomo-1.94.4/shlock.pl
--- majordomo-1.94.4.orig/shlock.pl Mon Mar 10 18:01:15 1997
+++ majordomo-1.94.4/shlock.pl Wed Jan 5 11:58:39 2000
@@ -1,3 +1,4 @@
+#!/usr/bin/perl
# PERL implementation of Erik E. Fair's 'shlock' (from the NNTP distribution)
# Ported by Brent Chapman <Brent@GreatCircle.COM>
diff -ur majordomo-1.94.4.orig/wrapper.c majordomo-1.94.4/wrapper.c
--- majordomo-1.94.4.orig/wrapper.c Wed Aug 27 17:01:12 1997
+++ majordomo-1.94.4/wrapper.c Wed Jan 5 11:58:39 2000
@@ -15,6 +15,8 @@
#include <stdio.h>
#include <sysexits.h>
+#include <getopt.h>
+#include <syslog.h>
#if defined(sun) && defined(sparc)
#include <stdlib.h>
@@ -26,8 +28,8 @@
# define STRCHR(s,c) strchr(s,c)
#endif
-#ifndef BIN
-# define BIN "/usr/local/mail/majordomo"
+#ifndef SBIN
+# define SBIN "/usr/local/mail/majordomo/sbin"
#endif
#ifndef PATH
@@ -57,6 +59,15 @@
int new_env_size = 7; /* to prevent overflow problems */
+static char * progname;
+
+static void * domalloc(unsigned int size);
+static void check_string(const char *);
+
+#define die() die_insecure(__LINE__)
+static void die_insecure(int line);
+
+int
main(argc, argv, env)
int argc;
char * argv[];
@@ -66,24 +77,76 @@
char * prog;
int e, i;
+ progname = strrchr(argv[0], '/');
+ if (progname || *++progname == '\0')
+ progname = argv[0];
+
if (argc < 2) {
- fprintf(stderr, "USAGE: %s program [<arg> ...]\n", argv[0]);
+ fprintf(stderr, "USAGE: %s program [<arg> ...]\n", progname);
exit(EX_USAGE);
}
+ /* Eat up argv[0] */
+ argv++; argc--;
+
/* if the command contains a /, then don't allow it */
- if (STRCHR(argv[1], '/') != (char *) NULL) {
- /* this error message is intentionally cryptic */
- fprintf(stderr, "%s: error: insecure usage\n", argv[0]);
- exit(EX_NOPERM);
- }
+ if (STRCHR(argv[0], '/') != (char *) NULL)
+ die();
- if ((prog = (char *) malloc(strlen(BIN) + strlen(argv[1]) + 2)) == NULL) {
- fprintf(stderr, "%s: error: malloc failed\n", argv[0]);
- exit(EX_OSERR);
+ /* Don't let non-privileged users specify arguments except
+ * -l listname
+ * -h hostname
+ */
+ if (getuid()) {
+ char **_argv, *listname = NULL, *hostname = NULL;
+ int c;
+
+ while ((c = getopt(argc, argv, "l:h:")) != -1) {
+ if (c == 'l')
+ listname = optarg;
+ else if (c == 'h')
+ hostname = optarg;
+ else
+ die();
+
+ /* Sanity check */
+ check_string(optarg);
+ }
+
+ /* Any remaining arguments?
+ * - Refuse to accept additional args except for resend
+ * - Make sure these cannot be options (caller could
+ * sneak them past us using the -- separator
+ * - make sure it doesn't begin with @ (special meaning)
+ * */
+ if (optind < argc) {
+ if (strcmp(argv[0], "resend"))
+ die();
+ for (i = optind; i < argc; i++)
+ check_string(argv[i]);
+ }
+
+ /* Re-assemble new argv */
+ _argv = (char **) domalloc((argc + 5) * sizeof(*argv));
+ _argv[0] = argv[0];
+ i = 1;
+ if (listname) {
+ _argv[i++] = "-l";
+ _argv[i++] = listname;
+ }
+ if (hostname) {
+ _argv[i++] = "-h";
+ _argv[i++] = hostname;
+ }
+ while (optind < argc)
+ _argv[i++] = argv[optind++];
+ _argv[i] = NULL;
+ argv = _argv;
+ argc = i;
}
- sprintf(prog, "%s/%s", BIN, argv[1]);
+ prog = (char *) domalloc(strlen(SBIN) + strlen(argv[0]) + 2);
+ sprintf(prog, "%s/%s", SBIN, argv[0]);
/* copy the "USER=" and "LOGNAME=" envariables into the new environment,
* if they exist.
@@ -117,7 +180,7 @@
#endif
extern int errno;
- fprintf(stderr, "%s: error setgroups failed errno %d", argv[0],
+ fprintf(stderr, "%s: error setgroups failed errno %d", progname,
errno);
}
}
@@ -137,13 +200,13 @@
#endif
if ((getuid() != geteuid()) || (getgid() != getegid())) {
- fprintf(stderr, "%s: error: Not running with proper UID and GID.\n", argv[0]);
+ fprintf(stderr, "%s: error: Not running with proper UID and GID.\n", progname);
fprintf(stderr, " Make certain that wrapper is installed setuid, and if so,\n");
fprintf(stderr, " recompile with POSIX flags.\n");
exit(EX_SOFTWARE);
}
- execve(prog, argv+1, new_env);
+ execve(prog, argv, new_env);
/* the exec should never return */
fprintf(stderr, "wrapper: Trying to exec %s failed: ", prog);
@@ -154,4 +217,51 @@
fprintf(stderr, " SHELL is %s,\n", SHELL);
fprintf(stderr, " MAJORDOMO_CF is %s\n", MAJORDOMO_CF);
exit(EX_OSERR);
+}
+
+static void *
+domalloc(unsigned int size)
+{
+ void *ptr;
+
+ ptr = (void *) malloc(size);
+ if (ptr == NULL) {
+ fprintf(stderr, "%s: error: malloc failed\n", progname);
+ exit(EX_OSERR);
+ }
+ return ptr;
+}
+
+/*
+ * Sanity-check a string.
+ */
+static void
+check_string(const char *s)
+{
+ char c;
+
+ /* Special cases: argument must not begin with characters
+ * that have special meaning on the command line:
+ * We include + because some versions of gnu getopt used
+ * to accept +longopt
+ */
+ if (strchr("-+@", s[0]))
+ die();
+
+ while ((c = *s++) != '\0') {
+ if (!isalnum(c) && !strchr("-._:+=", c))
+ die();
+ }
+}
+
+static void
+die_insecure(int line)
+{
+ /* this error message is intentionally cryptic */
+ fprintf(stderr, "%s: error: insecure usage\n", progname);
+
+ openlog(progname, LOG_NDELAY, LOG_MAIL);
+ syslog(LOG_WARNING,
+ "insecure usage (wrapper.c:%d); invoked by uid %d", line, getuid());
+ exit(EX_NOPERM);
}
Follow-Ups:
|
|
