You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apache-bugdb@apache.org by Jason Riedy <ej...@cise.ufl.edu> on 1997/07/26 09:00:03 UTC

suexec/921: Uses cwd before filling it in, doesn't use syslog

>Number:         921
>Category:       suexec
>Synopsis:       Uses cwd before filling it in, doesn't use syslog
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    apache (Apache HTTP Project)
>State:          open
>Class:          sw-bug
>Submitter-Id:   apache
>Arrival-Date:   Sat Jul 26 00:00:03 1997
>Originator:     ejr@cise.ufl.edu
>Organization:
apache
>Release:        1.2.1
>Environment:
SunOS trail 5.5 Generic_103093-12 sun4m sparc SUNW,SPARCstation-10
shouldn't matter...
>Description:
Yeah, I should separate these, but I'm lazy.

First: There are multiple references to cwd before it's set.

Second (how I noticed): suexec doesn't have a syslog option.  It's
kinda important for serious breakins.  

I picked LOG_INFO as the type of entry to show up where we have cgiwrap 
messages going.  Also, I use ../src/conf.h and ../src/util_snprintf.o
directly.  I changed the LOG_EXEC directive to USE_LOGFILE...  Twiddled
a message or two...  Can't remember the other changes...  Could probably
use the config info to determine if you have syslog, but you're changing
the error setup anyways...

Third:  You were using unguarded printfs, etc.  That's bad.  Buffer over-
flow; game over...  I think I cleared out all the possible run-time 
overflows by using snprintf.  There is the printf(...SAFE_PATH...), but
that should be OK.
>How-To-Repeat:
Try to print something with cwd to syslog...  ;)
>Fix:
Unified diff to follow...

--- suexec.c.orig       Sat Jul 26 02:40:21 1997
+++ suexec.c    Sat Jul 26 02:29:02 1997
@@ -69,6 +69,8 @@
 
 #include "suexec.h"
 
+#include "../src/conf.h"
+
 #include <sys/param.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -81,6 +83,10 @@
 #include <time.h>
 #include <sys/stat.h>
 
+#ifdef USE_SYSLOG
+#include <syslog.h>
+#endif
+
 #if defined(PATH_MAX)
 #define AP_MAXPATH PATH_MAX
 #elif defined(MAXPATHLEN)
@@ -92,7 +98,12 @@
 #define AP_ENVBUF 256
 
 extern char **environ;
+#ifdef USE_SYSLOG
+static int syslog_opened_p = 0;
+#endif /* USE_SYSLOG */
+#ifdef USE_LOGFILE
 static FILE *log;
+#endif /* USE_LOGFILE */
 
 char *safe_env_lst[] =
 {
@@ -140,35 +151,53 @@
 {
     time_t timevar;
     struct tm *lt;
+    char buff[BUFFLEN + 10];
+    int c_len;
 
+#ifdef USE_SYSLOG
+    if (!syslog_opened_p) {
+        syslog_opened_p = 1;
+        openlog ("suexec", LOG_PID | LOG_NOWAIT, LOG_DAEMON);
+    }
+#endif /* USE_SYSLOG */
+#ifdef USE_LOGFILE
     if (!log)
-       if ((log = fopen(LOG_EXEC, "a")) == NULL) {
+       if ((log = fopen(USE_LOGFILE, "a")) == NULL) {
            fprintf(stderr, "failed to open log file\n");
            perror("fopen");
            exit(1);
        }
+#endif /* USE_LOGFILE */
 
     time(&timevar);
     lt = localtime(&timevar);
     
-    fprintf(log, "[%.2d:%.2d:%.2d %.2d-%.2d-%.2d]: ", lt->tm_hour, lt->tm_min,
-           lt->tm_sec, lt->tm_mday, (lt->tm_mon + 1), lt->tm_year);
+    c_len = ap_snprintf (buff, BUFFLEN, "[%.2d:%.2d:%.2d %.2d-%.2d-%.2d]: ", 
+                         lt->tm_hour, lt->tm_min, lt->tm_sec, lt->tm_mday, 
+                         (lt->tm_mon + 1), lt->tm_year);
     
-    vfprintf(log, fmt, ap);
+    ap_vsnprintf (&buff[c_len], BUFFLEN - c_len, fmt, ap);
 
+#ifdef USE_SYSLOG
+    syslog (LOG_INFO, "%s", buff);
+#endif /* USE_SYSLOG */
+
+#ifdef USE_LOGFILE
+    fprintf (log, "%s", buff);
     fflush(log);
+#endif /* USE_LOGFILE */
     return;
 }
 
 void log_err(const char *fmt, ...)
 {
-#ifdef LOG_EXEC
+#if defined(USE_SYSLOG) | defined (USE_LOGFILE)
     va_list     ap;
 
     va_start(ap, fmt);
     err_output(fmt, ap);
     va_end(ap);
-#endif /* LOG_EXEC */
+#endif /* USE_SYSLOG | USE_LOGFILE */
     return;
 }
 
@@ -243,6 +272,14 @@
     target_gname = argv[2];
     cmd = argv[3];
 
+    /* 
+     * Determine the CWD here rather than after it's used.
+     */
+    if (getcwd(cwd, AP_MAXPATH) == NULL) {
+        log_err("cannot get current working directory\n");
+        exit(111);
+    }
+
     /*
      * Check existence/validity of the UID of the user
      * running this program.  Error out if invalid.
@@ -323,10 +360,10 @@
      * Log the transaction here to be sure we have an open log 
      * before we setuid().
      */
-    log_err("uid: (%s/%s) gid: (%s/%s) %s\n",
+    log_err("uid: (%s/%s) gid: (%s/%s) %s/%s\n",
              target_uname, actual_uname,
              target_gname, actual_gname,
-             cmd);
+             cwd, cmd);
 
     /*
      * Error out if attempt is made to execute as root or as
@@ -375,10 +412,12 @@
      * Use chdir()s and getcwd()s to avoid problems with symlinked
      * directories.  Yuck.
      */
+    /* Um, cwd has already been referenced...  Moved this up...
     if (getcwd(cwd, AP_MAXPATH) == NULL) {
         log_err("cannot get current working directory\n");
         exit(111);
     }
+    */
     
     if (userdir) {
         if (((chdir(target_homedir)) != 0) ||
@@ -463,12 +502,18 @@
 
     clean_env();
 
+#ifdef USE_SYSLOG
+    closelog ();
+#endif
+
+#ifdef USE_LOGFILE
     /* 
      * Be sure to close the log file so the CGI can't
      * mess with it.  If the exec fails, it will be reopened 
      * automatically when log_err is called.
      */
     fclose(log);
+#endif /* USE_LOGFILE */
     log = NULL;
     
     /*
--- suexec.h.orig       Sat Jul 26 02:40:26 1997
+++ suexec.h    Sat Jul 26 02:12:33 1997
@@ -80,7 +80,7 @@
  *            for suEXEC.  For most systems, 100 is common.
  */
 #ifndef GID_MIN
-#define GID_MIN 100
+#define GID_MIN 11
 #endif
 
 /*
@@ -108,22 +108,34 @@
 #define USERDIR_SUFFIX "public_html"
 #endif
 
+/* 
+ * BUFFLEN -- Length of error message buffer.  All messages longer than
+ *            this will be truncated.
+ */
+#define BUFFLEN 255
+
 /*
- * LOG_EXEC -- Define this as a filename if you want all suEXEC
- *             transactions and errors logged for auditing and
- *             debugging purposes.
+ * USE_LOGFILE -- Define this as a filename if you want all suEXEC
+ *                transactions and errors logged for auditing and
+ *                debugging purposes.
  */
-#ifndef LOG_EXEC
-#define LOG_EXEC "/usr/local/etc/httpd/logs/cgi.log" /* Need me? */
+#ifndef USE_LOGFILE
+/*#define USE_LOGFILE "/usr/local/etc/httpd/logs/cgi.log" /* Need me? */
+#define USE_LOGFILE "/var/log/suexec.log"
 #endif
 
 /*
+ * USE_SYSLOG -- Define this to log to LOG_INFO as "suexec".
+ */
+#define USE_SYSLOG
+
+/*
  * DOC_ROOT -- Define as the DocumentRoot set for Apache.  This
  *             will be the only hierarchy (aside from UserDirs)
  *             that can be used for suEXEC behavior.
  */
 #ifndef DOC_ROOT
-#define DOC_ROOT "/usr/local/etc/httpd/htdocs"
+#define DOC_ROOT "/cise/web/htdocs"
 #endif
 
 /*
--- Makefile.orig       Sat Jul 26 02:48:25 1997
+++ Makefile    Sat Jul 26 02:20:08 1997
@@ -7,7 +7,7 @@
 OPTIM=-O2
 CFLAGS1= -DSOLARIS2 -DSTATUS
 INCLUDES1= -Iregex
-LIBS1= -lsocket -lnsl
+LIBS1= -lsocket -lnsl 
 LFLAGS1=
 BROKEN_BPRINTF_FLAGS=
 REGLIB=regex/libregex.a
@@ -46,6 +46,9 @@
 
 logresolve: logresolve.c
        $(CC) $(INCLUDES) $(CFLAGS) logresolve.c -o logresolve $(LIBS)
+
+suexec:        suexec.c
+       $(CC) $(INCLUDES) $(CFLAGS) suexec.c -o suexec ../src/util_snprintf.o $(
LIBS)
 
 clean:
        rm -f $(TARGETS)
%0
>Audit-Trail:
>Unformatted: