Message-Id: <199706230600.BAA08008@sierra.zyzzyva.com>
To: new-httpd@apache.org
Subject: Re: [PATCH] various security problems 
In-reply-to: dgaudet's message of Sun, 22 Jun 1997 20:52:50 -0700.
         <Pine.LNX.3.95dg3.970622203628.4225A-100000@benchlark.arctic.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Date: Mon, 23 Jun 1997 01:00:20 -0500
From: Randy Terbush <randy@zyzzyva.com>
Sender: new-httpd-owner@apache.org
Precedence: bulk
Reply-To: new-httpd@apache.org

*sigh* I was hoping this email had been my imagination...

This sounds pretty serious, and by virtue of it being posted on 
this list, I think we had better move quickly to release a 1.2.1.

Are these problems worth creating a vendor initiated CERT advisory?

BTW - I could see some uses for allowing opens of UNIX domain sockets.


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