You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Dean Gaudet <dg...@arctic.org> on 1997/06/23 05:52:50 UTC

[PATCH] various security problems

Summary:  There's a bunch of ways to bypass the symlink restrictions, or
otherwise serve up any file on the system. 

1. Apache happily serves up any file system object, including pipes,
sockets, device files, whatever.  This allows users to do the hacks that
used to be popular with .plans and finger.  In particular it allows a user
to serve up any content from the system they want.  There's no reason to
consider any object other than files, symlinks, and directories. 

2. HeaderName, ReadmeName can be set to something containing ../ which
would allow the user to publish any file on the system.

3. mod_dir (serving headers, readme, and title scanning), mod_negotiation
(reading type map files), and mod_cern_meta (reading meta files)  violate
the symlinks permissions. 

4. The auth modules allow passwd/group files to be anywhere on the system. 
But I'm hard pressed to think of a way to abuse it. 

I fixed 1, 2, and 3.  The method I used to fix 3 is to do an extra
sub_req_lookup_file (or use the result of an already existing
sub_req_lookup).  This means, for example, that .var and .meta files
*must* be accessible to the client (i.e. result in a 200 response code if
someone accessed them directly).

I didn't test my fixes to mod_negotiation or mod_cern_meta.  The rest are
tested.

Third party modules probably have these problems as well.

We should consider starting a two week timer to a release of 1.2.1 with
this fix...

Dean

Index: http_request.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_request.c,v
retrieving revision 1.51
diff -c -3 -r1.51 http_request.c
*** http_request.c	1997/06/15 19:22:27	1.51
--- http_request.c	1997/06/23 03:46:50
***************
*** 85,90 ****
--- 85,108 ----
   * they change, all the way down.
   */
  
+ 
+ /*
+  * We don't want people able to serve up pipes, or unix sockets, or other
+  * scary things.  Note that symlink tests are performed later.
+  */
+ static int check_safe_file(request_rec *r)
+ {
+     if (r->finfo.st_mode == 0		/* doesn't exist */
+ 	|| S_ISDIR (r->finfo.st_mode)
+ 	|| S_ISREG (r->finfo.st_mode)
+ 	|| S_ISLNK (r->finfo.st_mode)) {
+ 	return OK;
+     }
+     log_reason("object is not a file, directory or symlink", r->filename, r);
+     return HTTP_FORBIDDEN;
+ }
+ 
+ 
  int check_symlinks (char *d, int opts)
  { 
  #if defined(__EMX__) || defined(WIN32)
***************
*** 310,320 ****
      if (res != OK) {
  	return res;
      }
!     
      if (test_filename[strlen(test_filename)-1] == '/')
  	--num_dirs;
  
!     if (S_ISDIR (r->finfo.st_mode)) ++num_dirs;
  
      for (i = 1; i <= num_dirs; ++i) {
          core_dir_config *core_dir =
--- 328,344 ----
      if (res != OK) {
  	return res;
      }
! 
!     if ((res = check_safe_file(r))) {
! 	return res;
!     }
! 
      if (test_filename[strlen(test_filename)-1] == '/')
  	--num_dirs;
  
!     if (S_ISDIR (r->finfo.st_mode)) {
! 	++num_dirs;
!     }
  
      for (i = 1; i <= num_dirs; ++i) {
          core_dir_config *core_dir =
***************
*** 667,672 ****
--- 699,709 ----
  	rnew->filename = make_full_path (rnew->pool, fdir, new_file);
  	if (stat (rnew->filename, &rnew->finfo) < 0) {
  	    rnew->finfo.st_mode = 0;
+ 	}
+ 
+ 	if ((res = check_safe_file(rnew))) {
+ 	    rnew->status = res;
+ 	    return rnew;
  	}
  
  	rnew->per_dir_config = r->per_dir_config;
Index: mod_cern_meta.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_cern_meta.c,v
retrieving revision 1.10
diff -c -3 -r1.10 mod_cern_meta.c
*** mod_cern_meta.c	1997/03/07 14:15:39	1.10
--- mod_cern_meta.c	1997/06/23 03:46:51
***************
*** 131,136 ****
--- 131,137 ----
  #include <sys/stat.h>
  #include "util_script.h"
  #include "http_log.h"
+ #include "http_request.h"
  
  #define DEFAULT_METADIR		".web"
  #define DEFAULT_METASUFFIX	".meta"
***************
*** 242,247 ****
--- 243,249 ----
      FILE *f;   
      cern_meta_config *cmc ;
      int rv;
+     request_rec *rr;
  
      cmc = get_module_config (r->server->module_config,
                             &cern_meta_module); 
***************
*** 276,304 ****
  
      metafilename = pstrcat(r->pool, "/", scrap_book, "/", cmc->metadir, "/", real_file, cmc->metasuffix, NULL);
  
!     /*
!      * stat can legitimately fail for a bewildering number of reasons,
!      * only one of which implies the file isn't there.  A hardened
!      * version of this module should test for all conditions, but later...
       */
!     if (stat(metafilename, &meta_stat) == -1) {
! 	/* stat failed, possibly file missing */
  	return DECLINED;
!     };
! 
!     /*
!      * this check is to be found in other Jan/96 Apache code, I've
!      * not been able to find any corroboration in the man pages but
!      * I've been wrong before so I'll put it in anyway.  Never
!      * admit to being clueless...
!      */
!     if ( meta_stat.st_mode == 0 ) {
! 	/* stat failed, definately file missing */
! 	return DECLINED;
!     };
  
      f = pfopen (r->pool, metafilename, "r");
-     
      if (f == NULL) {
          log_reason("meta file permissions deny server access", metafilename, r);
          return FORBIDDEN;
--- 278,296 ----
  
      metafilename = pstrcat(r->pool, "/", scrap_book, "/", cmc->metadir, "/", real_file, cmc->metasuffix, NULL);
  
!     /* XXX: it sucks to require this subrequest to complete, because this
!      * means people must leave their meta files accessible to the world.
!      * A better solution might be a "safe open" feature of pfopen to avoid
!      * pipes, symlinks, and crap like that.
       */
!     rr = sub_req_lookup_file (metafilename, r);
!     if (rr->status != HTTP_OK) {
! 	destroy_sub_req (rr);
  	return DECLINED;
!     }
!     destroy_sub_req (rr);
  
      f = pfopen (r->pool, metafilename, "r");
      if (f == NULL) {
          log_reason("meta file permissions deny server access", metafilename, r);
          return FORBIDDEN;
Index: mod_dir.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_dir.c,v
retrieving revision 1.28
diff -c -3 -r1.28 mod_dir.c
*** mod_dir.c	1997/06/17 00:09:14	1.28
--- mod_dir.c	1997/06/23 03:46:53
***************
*** 168,178 ****
--- 168,184 ----
  }
  
  const char *add_header(cmd_parms *cmd, void *d, char *name) {
+     if (strchr (name, '/')) {
+ 	return "HeaderName cannot contain a /";
+     }
      push_item(((dir_config_rec *)d)->hdr_list, 0, NULL, cmd->path, name);
      return NULL;
  }
  
  const char *add_readme(cmd_parms *cmd, void *d, char *name) {
+     if (strchr (name, '/')) {
+ 	return "ReadmeName cannot contain a /";
+     }
      push_item(((dir_config_rec *)d)->rdme_list, 0, NULL, cmd->path, name);
      return NULL;
  }
***************
*** 414,420 ****
--- 420,428 ----
      FILE *f;
      struct stat finfo;
      int plaintext=0;
+     request_rec *rr;
  
+     /* XXX: this is a load of crap, it needs to do a full sub_req_lookup_uri */
      fn = make_full_path(r->pool, name, readme_fname);
      fn = pstrcat(r->pool, fn, ".html", NULL);
      if(stat(fn,&finfo) == -1) {
***************
*** 427,432 ****
--- 435,448 ----
          rputs("<PRE>\n", r);
      }
      else if (rule) rputs("<HR>\n", r);
+     /* XXX: when the above is rewritten properly, this necessary security
+      * check will be redundant. -djg */
+     rr = sub_req_lookup_file (fn, r);
+     if (rr->status != HTTP_OK) {
+ 	destroy_sub_req (rr);
+ 	return 0;
+     }
+     destroy_sub_req (rr);
      if(!(f = pfopen(r->pool,fn,"r")))
          return 0;
      if (!plaintext)
***************
*** 468,473 ****
--- 484,492 ----
      FILE *thefile = NULL;
      int x,y,n,p;
  
+     if (r->status != HTTP_OK) {
+ 	return NULL;
+     }
      if (r->content_type && !strcmp(r->content_type,"text/html") && !r->content_encoding) {
          if(!(thefile = pfopen(r->pool, r->filename,"r")))
              return NULL;
Index: mod_negotiation.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_negotiation.c,v
retrieving revision 1.42
diff -c -3 -r1.42 mod_negotiation.c
*** mod_negotiation.c	1997/06/17 00:09:14	1.42
--- mod_negotiation.c	1997/06/23 03:46:59
***************
*** 645,661 ****
      return cp;
  }
  
! int read_type_map (negotiation_state *neg, char *map_name)
  {
      request_rec *r = neg->r;
!     FILE *map = pfopen (neg->pool, map_name, "r");
! 
      char buffer[MAX_STRING_LEN];
      enum header_state hstate;
      struct var_rec mime_info;
      
      if (map == NULL) {
!         log_reason("cannot access type map file", map_name, r);
  	return FORBIDDEN;
      }
  
--- 645,664 ----
      return cp;
  }
  
! static int read_type_map (negotiation_state *neg, request_rec *rr)
  {
      request_rec *r = neg->r;
!     FILE *map;
      char buffer[MAX_STRING_LEN];
      enum header_state hstate;
      struct var_rec mime_info;
      
+     if (rr->status != HTTP_OK) {
+ 	return rr->status;
+     }
+     map = pfopen (neg->pool, rr->filename, "r");
      if (map == NULL) {
!         log_reason("cannot access type map file", rr->filename, r);
  	return FORBIDDEN;
      }
  
***************
*** 783,789 ****
  	    closedir(dirp);
  	    
  	    neg->avail_vars->nelts = 0;
! 	    return read_type_map (neg, sub_req->filename);
  	}
  	
  	/* Have reasonable variant --- gather notes.
--- 786,792 ----
  	    closedir(dirp);
  	    
  	    neg->avail_vars->nelts = 0;
! 	    return read_type_map (neg, sub_req);
  	}
  	
  	/* Have reasonable variant --- gather notes.
***************
*** 1853,1859 ****
      
      char *udir;
      
!     if ((res = read_type_map (neg, r->filename))) return res;
      
      maybe_add_default_encodings(neg, 0);
      
--- 1856,1862 ----
      
      char *udir;
      
!     if ((res = read_type_map (neg, r))) return res;
      
      maybe_add_default_encodings(neg, 0);
      


Re: [PATCH] various security problems

Posted by Dean Gaudet <dg...@arctic.org>.
I wonder if we couldn't get further with something that works on absolute
paths.  i.e. when symlinks are followed, start processing the absolute
path, or provide directives that match the absolute path.  In the end
though we're doing a lot of work for little gain ... we need to protect so
much of the code, ugh. 

Your module is simple enough, and catches another trouble case.  But it
just begs for extension ... like "disallow uids < 100" :)  I'm happy
including it as is though. 

Dean

On Thu, 3 Jul 1997, Lou D. Langholtz wrote:

> Dean Gaudet wrote:
> > 
> > Summary:  There's a bunch of ways to bypass the symlink restrictions, or
> > otherwise serve up any file on the system.
> > . . .
> 
> Just a reminder on an old post I made...
> 
> On this thread, FollowSymlinksIfOwnersMatch can also be circumvented by
> users by telling there sys-admin to restore one of there web directories
> containing some symlinks which often doesn't preserve the symlink owner.
> Now they got root owned symlinks which is just perfect for pointing at /
> and voila, they can export the whole filesystem tree (just about).
> 
> If anybody wants additional protection against Symlink dangers I wrote a
> simple module a while back and put it at
> <http://www.eng.utah.edu/~ldl/apache/modules/disallow_id/>. I've also
> recently updated it to compile more cleanly with 1.2.0 as well as 1.1.*
> 


Re: [PATCH] various security problems

Posted by "Lou D. Langholtz" <ld...@usi.utah.edu>.
Dean Gaudet wrote:
> 
> Summary:  There's a bunch of ways to bypass the symlink restrictions, or
> otherwise serve up any file on the system.
> . . .

Just a reminder on an old post I made...

On this thread, FollowSymlinksIfOwnersMatch can also be circumvented by
users by telling there sys-admin to restore one of there web directories
containing some symlinks which often doesn't preserve the symlink owner.
Now they got root owned symlinks which is just perfect for pointing at /
and voila, they can export the whole filesystem tree (just about).

If anybody wants additional protection against Symlink dangers I wrote a
simple module a while back and put it at
<http://www.eng.utah.edu/~ldl/apache/modules/disallow_id/>. I've also
recently updated it to compile more cleanly with 1.2.0 as well as 1.1.*