httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Randy Terbush <ra...@zyzzyva.com>
Subject [PATCH] a different approach to setuid scripts
Date Wed, 18 Jun 1997 22:46:08 GMT

He has chosen the path of setting effective UIDs, which was 
originally not acceptable. My feelings haven't changed much in that 
regard. Would someone else like to comment?

------- Forwarded Message

To: Randy Terbush <randy@zyzzyva.com>
Subject: Re: Possible security hole in suEXEC 
In-Reply-To: Your message of "Wed, 18 Jun 1997 16:54:17 CDT."
             <199706182154.QAA01708@sierra.zyzzyva.com> 
Date: Wed, 18 Jun 1997 18:19:29 EDT
From: Monty <xiphmont@MIT.EDU>


No problem.

Monty

::::: Included File :::::

diff -wPrc apache_1.2b10/src/Configuration apache_1.2b10monty/src/Configuration
*** apache_1.2b10/src/Configuration	Mon Apr 28 22:11:16 1997
- --- apache_1.2b10monty/src/Configuration	Wed Jun 18 15:06:17 1997
***************
*** 41,47 ****
  # Settings here have priority; If not set, Configure will attempt to guess
  # the C compiler, and set OPTIM to '-O2'
  #
! EXTRA_CFLAGS=
  EXTRA_LFLAGS=
  EXTRA_LIBS=
  EXTRA_INCLUDES=
- --- 41,47 ----
  # Settings here have priority; If not set, Configure will attempt to guess
  # the C compiler, and set OPTIM to '-O2'
  #
! EXTRA_CFLAGS=	-DSERVER_SUBVERSION="\"Monty-ScriptUser/1.0-alpha\""
  EXTRA_LFLAGS=
  EXTRA_LIBS=
  EXTRA_INCLUDES=
diff -wPrc apache_1.2b10/src/alloc.c apache_1.2b10monty/src/alloc.c
*** apache_1.2b10/src/alloc.c	Thu Apr 24 16:35:18 1997
- --- apache_1.2b10monty/src/alloc.c	Wed Jun 18 13:16:05 1997
***************
*** 764,770 ****
      cleanup_pool_for_exec (p);
  }
  
! void cleanup_for_exec()
  {
    block_alarms();
    cleanup_pool_for_exec (permanent_pool);
- --- 764,770 ----
      cleanup_pool_for_exec (p);
  }
  
! void alloc_cleanup_for_exec()
  {
    block_alarms();
    cleanup_pool_for_exec (permanent_pool);
diff -wPrc apache_1.2b10/src/alloc.h apache_1.2b10monty/src/alloc.h
*** apache_1.2b10/src/alloc.h	Thu Apr 24 16:35:19 1997
- --- apache_1.2b10monty/src/alloc.h	Wed Jun 18 13:19:06 1997
***************
*** 87,93 ****
   * buffers, *don't* wait for subprocesses, and *don't* free any memory.
   */
  
! void cleanup_for_exec ();
  
  /* routines to allocate memory from an pool... */
  
- --- 87,93 ----
   * buffers, *don't* wait for subprocesses, and *don't* free any memory.
   */
  
! void alloc_cleanup_for_exec ();
  
  /* routines to allocate memory from an pool... */
  
diff -wPrc apache_1.2b10/src/http_main.c apache_1.2b10monty/src/http_main.c
*** apache_1.2b10/src/http_main.c	Mon Apr 28 19:39:01 1997
- --- apache_1.2b10monty/src/http_main.c	Tue Jun 17 00:10:52 1997
***************
*** 1430,1437 ****
  	exit (1);
      }
  #endif
!     if (setgid(group_id) == -1) {
! 	log_unixerr("setgid", NULL, "unable to set group id", server_conf);
  	exit (1);
      }
  #endif 
- --- 1430,1437 ----
  	exit (1);
      }
  #endif
!     if (setegid(group_id) == -1) {
! 	log_unixerr("setegid", NULL, "unable to set group id", server_conf);
  	exit (1);
      }
  #endif 
***************
*** 1670,1682 ****
      /* Only try to switch if we're running as MANAGER.SYS */
      if (geteuid() == 1 && user_id > 1) {
          GETPRIVMODE();
!         if (setuid(user_id) == -1) {
              GETUSERMODE();
  #else
      /* Only try to switch if we're running as root */
!     if (!geteuid() && setuid(user_id) == -1) {
  #endif
!         log_unixerr("setuid", NULL, "unable to change uid", server_conf);
  	exit (1);
      }
  #ifdef MPE
- --- 1670,1682 ----
      /* Only try to switch if we're running as MANAGER.SYS */
      if (geteuid() == 1 && user_id > 1) {
          GETPRIVMODE();
!         if (seteuid(user_id) == -1) {
              GETUSERMODE();
  #else
      /* Only try to switch if we're running as root */
!     if (!geteuid() && seteuid(user_id) == -1) {
  #endif
!         log_unixerr("seteuid", NULL, "unable to change euid", server_conf);
  	exit (1);
      }
  #ifdef MPE
***************
*** 2409,2421 ****
        /* Only try to switch if we're running as MANAGER.SYS */
        if (geteuid() == 1 && user_id > 1) {
            GETPRIVMODE();
!           if (setuid(user_id) == -1) {
                GETUSERMODE();
  #else
        /* Only try to switch if we're running as root */
!       if(!geteuid() && setuid(user_id) == -1) {
  #endif
!           log_unixerr("setuid", NULL, "unable to change uid", server_conf);
            exit (1);
        }
  #ifdef MPE
- --- 2409,2421 ----
        /* Only try to switch if we're running as MANAGER.SYS */
        if (geteuid() == 1 && user_id > 1) {
            GETPRIVMODE();
!           if (seteuid(user_id) == -1) {
                GETUSERMODE();
  #else
        /* Only try to switch if we're running as root */
!       if(!geteuid() && seteuid(user_id) == -1) {
  #endif
!           log_unixerr("setuid", NULL, "unable to change euid", server_conf);
            exit (1);
        }
  #ifdef MPE
diff -wPrc apache_1.2b10/src/httpd.h apache_1.2b10monty/src/httpd.h
*** apache_1.2b10/src/httpd.h	Mon Apr 28 22:08:12 1997
- --- apache_1.2b10monty/src/httpd.h	Tue Jun 17 01:30:24 1997
***************
*** 703,709 ****
  uid_t uname2id(const char *name);
  gid_t gname2id(const char *name);
  int is_directory(const char *name);
! int can_exec(const struct stat *);     
  void chdir_file(const char *file);
       
  char *get_local_host(pool *);
- --- 703,709 ----
  uid_t uname2id(const char *name);
  gid_t gname2id(const char *name);
  int is_directory(const char *name);
! int can_exec(const struct stat *,uid_t uid,gid_t gid);     
  void chdir_file(const char *file);
       
  char *get_local_host(pool *);
diff -wPrc apache_1.2b10/src/mod_cgi.c apache_1.2b10monty/src/mod_cgi.c
*** apache_1.2b10/src/mod_cgi.c	Mon Apr 21 13:29:09 1997
- --- apache_1.2b10monty/src/mod_cgi.c	Wed Jun 18 14:02:28 1997
***************
*** 96,127 ****
      char *logname;
      long logbytes;
      int bufbytes;
! } cgi_server_conf;
  
  void *create_cgi_config (pool *p, server_rec *s)
  {
!     cgi_server_conf *c = 
!       (cgi_server_conf *)pcalloc (p, sizeof(cgi_server_conf));
  
      c->logname = NULL;
      c->logbytes = DEFAULT_LOGBYTES;
      c->bufbytes = DEFAULT_BUFBYTES;
  
      return c;
  }
  
  void *merge_cgi_config (pool *p, void *basev, void *overridesv)
  {
!     cgi_server_conf *base = (cgi_server_conf *)basev,
!       *overrides = (cgi_server_conf *)overridesv;
  
      return overrides->logname ? overrides : base;
  }
  
  const char *set_scriptlog (cmd_parms *cmd, void *dummy, char *arg) {
      server_rec *s = cmd->server;
!     cgi_server_conf *conf = 
!       (cgi_server_conf *)get_module_config(s->module_config, &cgi_module);
  
      conf->logname = arg;
      return NULL;
- --- 96,183 ----
      char *logname;
      long logbytes;
      int bufbytes;
!     int exec_uid;
!     int exec_gid;
! } cgi_sd_conf;
  
  void *create_cgi_config (pool *p, server_rec *s)
  {
!     cgi_sd_conf *c = 
!       (cgi_sd_conf *)pcalloc (p, sizeof(cgi_sd_conf));
! #ifdef DEBUG_CGI    
! #ifdef __EMX__
!     /* Under OS/2 need to use device con. */
!     FILE *dbg = fopen ("con", "w");
! #else    
!     FILE *dbg = fopen ("/dev/tty", "w");
! #endif    
! #endif
  
      c->logname = NULL;
      c->logbytes = DEFAULT_LOGBYTES;
      c->bufbytes = DEFAULT_BUFBYTES;
+     c->exec_uid=-1;
+     c->exec_gid=-1;
+ 
+ #ifdef DEBUG_CGI
+     fprintf (dbg, "**IDs exiting create: \n");
+     fprintf (dbg, "  uid:%d gid:%d\n",
+ 	     c->exec_uid,c->exec_gid);
+     fprintf (dbg, "  config:%x\n",c);
+     fclose(dbg);
+ #endif
  
      return c;
  }
  
+ void *create_cgi_dir_config (pool *p, char *d){
+   return(create_cgi_config(p,NULL));
+ }
+ 
  void *merge_cgi_config (pool *p, void *basev, void *overridesv)
  {
!     cgi_sd_conf *base = (cgi_sd_conf *)basev,
!       *overrides = (cgi_sd_conf *)overridesv;
! 
! #ifdef DEBUG_CGI    
! #ifdef __EMX__
!     /* Under OS/2 need to use device con. */
!     FILE *dbg = fopen ("con", "w");
! #else    
!     FILE *dbg = fopen ("/dev/tty", "w");
! #endif    
!     fprintf (dbg, "**IDs entering merge: \n");
!     fprintf (dbg, "  base_uid:%d base_gif:%d over_uid:%d over_gid:%d\n",
! 	     base->exec_uid,base->exec_gid,overrides->exec_uid,
! 	     overrides->exec_gid);
!     fclose(dbg);
! #endif
! 
!     if(overrides->exec_uid!=-1 || overrides->exec_gid!=-1){
!       base->exec_uid=overrides->exec_uid;
!       base->exec_gid=overrides->exec_gid;
!     }else{
!       overrides->exec_uid=base->exec_uid;
!       overrides->exec_gid=base->exec_gid;
!     }
  
      return overrides->logname ? overrides : base;
  }
  
+ const char *set_script_user (cmd_parms *cmd, cgi_sd_conf *conf, char *arg) {
+     conf->exec_uid=uname2id(arg);
+     return NULL;
+ }
+ 
+ const char *set_script_group (cmd_parms *cmd, cgi_sd_conf *conf, char *arg) {
+     conf->exec_gid=gname2id(arg);
+     return NULL;
+ }
+ 
  const char *set_scriptlog (cmd_parms *cmd, void *dummy, char *arg) {
      server_rec *s = cmd->server;
!     cgi_sd_conf *conf = 
!       (cgi_sd_conf *)get_module_config(s->module_config, &cgi_module);
  
      conf->logname = arg;
      return NULL;
***************
*** 129,136 ****
  
  const char *set_scriptlog_length (cmd_parms *cmd, void *dummy, char *arg) {
      server_rec *s = cmd->server;
!     cgi_server_conf *conf = 
!       (cgi_server_conf *)get_module_config(s->module_config, &cgi_module);
  
      conf->logbytes = atol (arg);
      return NULL;
- --- 185,192 ----
  
  const char *set_scriptlog_length (cmd_parms *cmd, void *dummy, char *arg) {
      server_rec *s = cmd->server;
!     cgi_sd_conf *conf = 
!       (cgi_sd_conf *)get_module_config(s->module_config, &cgi_module);
  
      conf->logbytes = atol (arg);
      return NULL;
***************
*** 138,145 ****
  
  const char *set_scriptlog_buffer (cmd_parms *cmd, void *dummy, char *arg) {
      server_rec *s = cmd->server;
!     cgi_server_conf *conf = 
!       (cgi_server_conf *)get_module_config(s->module_config, &cgi_module);
  
      conf->bufbytes = atoi (arg);
      return NULL;
- --- 194,201 ----
  
  const char *set_scriptlog_buffer (cmd_parms *cmd, void *dummy, char *arg) {
      server_rec *s = cmd->server;
!     cgi_sd_conf *conf = 
!       (cgi_sd_conf *)get_module_config(s->module_config, &cgi_module);
  
      conf->bufbytes = atoi (arg);
      return NULL;
***************
*** 152,161 ****
    "the maximum length (in bytes) of the script debug log"},
  { "ScriptLogBuffer", set_scriptlog_buffer, NULL, RSRC_CONF, TAKE1,
    "the maximum size (in bytes) to record of a POST request"},
  { NULL}
  };
  
! static int log_scripterror(request_rec *r, cgi_server_conf *conf, int ret,
  		    char *error)
  {
      FILE *f;
- --- 208,222 ----
    "the maximum length (in bytes) of the script debug log"},
  { "ScriptLogBuffer", set_scriptlog_buffer, NULL, RSRC_CONF, TAKE1,
    "the maximum size (in bytes) to record of a POST request"},
+   /* Monty hack */
+ { "ScriptUser", set_script_user,NULL,RSRC_CONF|ACCESS_CONF, TAKE1,
+     "the desired target uid of a CGI script"},
+ { "ScriptGroup", set_script_group,NULL,RSRC_CONF|ACCESS_CONF, TAKE1,
+     "the desired target gid of a CGI script"},
  { NULL}
  };
  
! static int log_scripterror(request_rec *r, cgi_sd_conf *conf, int ret,
  		    char *error)
  {
      FILE *f;
***************
*** 182,188 ****
      return ret;
  }
  
! static int log_script(request_rec *r, cgi_server_conf *conf, int ret,
  	       char *dbuf, char *sbuf, FILE *script_in, FILE *script_err)
  {
      table *hdrs_arr = r->headers_in;
- --- 243,249 ----
      return ret;
  }
  
! static int log_script(request_rec *r, cgi_sd_conf *conf, int ret,
  	       char *dbuf, char *sbuf, FILE *script_in, FILE *script_err)
  {
      table *hdrs_arr = r->headers_in;
***************
*** 317,323 ****
       * NB only ISINDEX scripts get decoded arguments.
       */
      
!     cleanup_for_exec();
      
      call_exec(r, argv0, env, 0);
  
- --- 378,421 ----
       * NB only ISINDEX scripts get decoded arguments.
       */
  
! 
!     /* Monty's CGI ScriptUser/Script Group hack; call_exec does the
!        actual setting from the globals; we've already forked, so go
!        ahead and clobber state */
!     {
!       cgi_sd_conf *conf = get_module_config(r->per_dir_config,&cgi_module);
! 
! #ifdef DEBUG_CGI
!       fprintf (dbg, "**IDs before exec: \n");
!       fprintf (dbg, "  uid:%d  euid:%d  gid:%d  egid:%d\n",(int)getuid(),
! 	       (int)geteuid(),(int)getgid(),(int)getegid());
!       fprintf (dbg, "  user_id:%d  group_id:%d\n",(int)user_id,(int)group_id);
!       fprintf (dbg, "  request:%x  dir_config:%x\n",r,conf);
! #endif    
! 
!       /* First, some paranoia; check consistency */
! 
!       check_uidgid(r->server);
! 
!       /* Set what we want to use for the exec */
! 
!       user_id=(conf->exec_uid<1)?user_id:conf->exec_uid;
!       group_id=(conf->exec_gid<1)?group_id:conf->exec_gid;
!       make_exec_safe(r->server);
! 
! #ifdef DEBUG_CGI
!       fprintf (dbg, "**IDs after exec prep: \n");
!       fprintf (dbg, "  uid:%d  euid:%d  gid:%d  egid:%d\n",(int)getuid(),
! 	       (int)geteuid(),(int)getgid(),(int)getegid());
!       fprintf (dbg, "  user_id:%d  group_id:%d\n",(int)user_id,(int)group_id);
! #endif    
!     }
! 
!     /* We already called (above) the contents of the new cleanup_for_exec()
!        preceeding alloc_cleanup_for_exec() [which is the old 
!        cleanup_for_exec()] */
! 
!     alloc_cleanup_for_exec();
      
      call_exec(r, argv0, env, 0);
  
***************
*** 346,353 ****
      char argsbuffer[HUGE_STRING_LEN];
      int is_included = !strcmp (r->protocol, "INCLUDED");
      void *sconf = r->server->module_config;
!     cgi_server_conf *conf =
! 	(cgi_server_conf *)get_module_config(sconf, &cgi_module);
  
      struct cgi_child_stuff cld;
      pid_t child_pid;
- --- 444,453 ----
      char argsbuffer[HUGE_STRING_LEN];
      int is_included = !strcmp (r->protocol, "INCLUDED");
      void *sconf = r->server->module_config;
!     cgi_sd_conf *conf =
! 	(cgi_sd_conf *)get_module_config(sconf, &cgi_module);
!     cgi_sd_conf *dirconf = 
!         (cgi_sd_conf *)get_module_config(r->per_dir_config,&cgi_module);
  
      struct cgi_child_stuff cld;
      pid_t child_pid;
***************
*** 393,399 ****
  			       "script not found or unable to stat");
  #endif
      if (!suexec_enabled) {
!         if (!can_exec(&r->finfo))
              return log_scripterror(r, conf, FORBIDDEN,
                                     "file permissions deny server execution");
      }
- --- 493,502 ----
  			       "script not found or unable to stat");
  #endif
      if (!suexec_enabled) {
!         int uid=dirconf->exec_uid;
!         int gid=dirconf->exec_gid;
!         if (!can_exec(&r->finfo,(uid==-1)?user_id:uid,
! 		      (gid==-1?group_id:gid)))
              return log_scripterror(r, conf, FORBIDDEN,
                                     "file permissions deny server execution");
      }
***************
*** 557,564 ****
  module cgi_module = {
     STANDARD_MODULE_STUFF,
     NULL,			/* initializer */
!    NULL,			/* dir config creater */
!    NULL,			/* dir merger --- default is to override */
     create_cgi_config,		/* server config */
     merge_cgi_config,	       	/* merge server config */
     cgi_cmds,			/* command table */
- --- 660,667 ----
  module cgi_module = {
     STANDARD_MODULE_STUFF,
     NULL,			/* initializer */
!    create_cgi_dir_config,       /* dir config creater */
!    merge_cgi_config,		/* dir merger --- default is to override */
     create_cgi_config,		/* server config */
     merge_cgi_config,	       	/* merge server config */
     cgi_cmds,			/* command table */
diff -wPrc apache_1.2b10/src/util.c apache_1.2b10monty/src/util.c
*** apache_1.2b10/src/util.c	Fri Apr 11 21:24:59 1997
- --- apache_1.2b10monty/src/util.c	Tue Jun 17 00:43:42 1997
***************
*** 937,943 ****
      return (x ? 1 : 0);  /* If the first character is ':', it's broken, too */
  }
  
! int can_exec(const struct stat *finfo) {
  #ifdef MULTIPLE_GROUPS
    int cnt;
  #endif
- --- 937,943 ----
      return (x ? 1 : 0);  /* If the first character is ':', it's broken, too */
  }
  
! int can_exec(const struct stat *finfo,uid_t uid,gid_t gid) {
  #ifdef MULTIPLE_GROUPS
    int cnt;
  #endif
***************
*** 945,954 ****
      /* OS/2 dosen't have Users and Groups */
      return 1;
  #else    
!     if(user_id == finfo->st_uid)
          if(finfo->st_mode & S_IXUSR)
              return 1;
!     if(group_id == finfo->st_gid)
          if(finfo->st_mode & S_IXGRP)
              return 1;
  #ifdef MULTIPLE_GROUPS
- --- 945,954 ----
      /* OS/2 dosen't have Users and Groups */
      return 1;
  #else    
!     if(uid == finfo->st_uid)
          if(finfo->st_mode & S_IXUSR)
              return 1;
!     if(gid == finfo->st_gid)
          if(finfo->st_mode & S_IXGRP)
              return 1;
  #ifdef MULTIPLE_GROUPS
diff -wPrc apache_1.2b10/src/util_script.c apache_1.2b10monty/src/util_script.c
*** apache_1.2b10/src/util_script.c	Mon Apr 28 21:45:52 1997
- --- apache_1.2b10monty/src/util_script.c	Wed Jun 18 14:10:57 1997
***************
*** 439,444 ****
- --- 439,531 ----
  }
  #endif
  
+ void check_uidgid(server_rec *s)
+ {
+   /* The redundancy is intentional */
+   
+   if(geteuid()!=user_id || getegid()!=group_id){
+     if(s)
+       log_unixerr("uid/gid",NULL,
+ 		  "Internal error: uid/euid/gid/egid security compromised!\n"
+ 		  "                crashing child process NOW!\n",
+ 		  s);
+     else
+       fprintf(stderr,
+ 	      "uid/gid: Internal error: inconsistent uid/euid/gid/egid\n"
+ 	      "         avoiding exec\n");
+ 
+     exit(0);
+   }
+ }
+ 
+ void make_exec_safe(server_rec *s)
+ {
+   /* Monty's euid/egid hack that lets us do ScriptUser and ScriptGroup
+      must now be made safe */
+   
+     /* disallow root */
+ 
+     if(!user_id || !group_id){
+       if(s)
+ 	log_unixerr("uid/gid",NULL,
+ 		    "Exec error: execing a child with real uid/gid\n"    
+ 		    "            set to root is not permitted.\n",
+ 		    s);
+       else
+ 	fprintf(stderr,
+ 		"uid/gid: execing with uid/gid root is not allowed.\n");
+       
+       exit(0);
+     }
+ 
+     /* Set what we really want for the exec */
+ 
+     seteuid(0);
+     setegid(0);
+     setgid(group_id);
+     setuid(user_id);
+     
+     if(getgid()!=group_id || getegid()!=group_id){
+       if(s)
+ 	log_unixerr("setgid",NULL,
+ 		    "Internal error: failed to set real/saved gid before exec\n"
+ 		    "                possible security compromise: "
+ 		    "crashing child process NOW!\n",
+ 		    s);
+       else
+ 	fprintf(stderr,
+ 		"setgid: Internal error: Failed to set gid; avoiding exec.\n");
+       exit(0);
+     }
+     if(getuid()!=user_id || geteuid()!=user_id){
+       if(s)
+ 	log_unixerr("setuid",NULL,
+ 		    "Internal error: failed to set real/saved gid before exec\n"
+ 		    "                possible security compromise: "
+ 		    "crashing child process NOW!\n",
+ 		    s);
+       else
+ 	fprintf(stderr,
+ 		"setgid: Internal error: Failed to set uid; avoiding exec.\n");
+ 
+       exit(0);
+     }
+ }
+ 
+ /* A wrapper for the old cleanup_for_exec that makes sure our uid/gid are safe.
+    This stuff didn't really belong in alloc.c :-) */
+ 
+ void cleanup_for_exec(){
+ 
+   /* Call our new stuff */
+ 
+   check_uidgid(NULL);
+   make_exec_safe(NULL);
+ 
+   /* Call the original */
+ 
+   alloc_cleanup_for_exec();
+ }
  
  void call_exec (request_rec *r, char *argv0, char **env, int shellcmd) 
  {
diff -wPrc apache_1.2b10/src/util_script.h apache_1.2b10monty/src/util_script.h
*** apache_1.2b10/src/util_script.h	Mon Apr 28 20:41:13 1997
- --- apache_1.2b10monty/src/util_script.h	Wed Jun 18 14:10:33 1997
***************
*** 66,69 ****
  int scan_script_header_err(request_rec *r, FILE *f, char *buffer);
  void send_size(size_t size, request_rec *r);
  void call_exec (request_rec *r, char *argv0, char **env, int shellcmd);
! 
- --- 66,71 ----
  int scan_script_header_err(request_rec *r, FILE *f, char *buffer);
  void send_size(size_t size, request_rec *r);
  void call_exec (request_rec *r, char *argv0, char **env, int shellcmd);
! void check_uidgid(server_rec *s);
! void make_exec_safe(server_rec *s);
! void cleanup_for_exec();

------- End of Forwarded Message




Mime
View raw message