From: dgaudet <dgaudet@unknown>
Date: Fri, 27 Jun 1997 02:21:22 +0000 (+0000)
Subject: Ugh I messed up the security fixes commit.
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=090b91dc72d0c6569429d05cf2474624c844afdb;p=thirdparty%2Fapache%2Fhttpd.git

Ugh I messed up the security fixes commit.
Reviewed by:
Submitted by:
Obtained from:


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/1.3@78387 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/APACHE_1_2_X/src/main/http_request.c b/APACHE_1_2_X/src/main/http_request.c
index 72574be4739..ee34deb51a4 100644
--- a/APACHE_1_2_X/src/main/http_request.c
+++ b/APACHE_1_2_X/src/main/http_request.c
@@ -85,6 +85,24 @@
  * 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)
 {
     struct stat lfi, fi;
@@ -310,11 +328,17 @@ int directory_walk (request_rec *r)
     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;
+    if (S_ISDIR (r->finfo.st_mode)) {
+	++num_dirs;
+    }
 
     for (i = 1; i <= num_dirs; ++i) {
         core_dir_config *core_dir =
@@ -399,8 +423,16 @@ int directory_walk (request_rec *r)
 
     r->per_dir_config = per_dir_defaults;
 
-    if ((res = check_symlinks (r->filename, allow_options(r))))
-    {
+    /* Symlink permissions are determined by the parent.  If the request is for
+     * a directory then applying the symlink test here would use the
+     * permissions of the directory as opposed to its parent.  Consider a
+     * symlink pointing to a dir with a .htaccess disallowing symlinks.  If you
+     * access /symlink (or /symlink/) you would get a 403 without this S_ISDIR
+     * test.  But if you accessed /symlink/index.html, for example, you would
+     * *not* get the 403.
+     */
+    if (!S_ISDIR (r->finfo.st_mode)
+	&& (res = check_symlinks (r->filename, allow_options(r)))) {
 	log_reason("Symbolic link not allowed", r->filename, r);
 	return res;
     }
@@ -669,6 +701,11 @@ request_rec *sub_req_lookup_file (const char *new_file, const request_rec *r)
 	    rnew->finfo.st_mode = 0;
 	}
 
+	if ((res = check_safe_file(rnew))) {
+	    rnew->status = res;
+	    return rnew;
+	}
+
 	rnew->per_dir_config = r->per_dir_config;
 
 	if ((res = check_symlinks (rnew->filename, allow_options (rnew)))) {
diff --git a/APACHE_1_2_X/src/modules/standard/mod_cern_meta.c b/APACHE_1_2_X/src/modules/standard/mod_cern_meta.c
index 321175f36c3..d05677c8c25 100644
--- a/APACHE_1_2_X/src/modules/standard/mod_cern_meta.c
+++ b/APACHE_1_2_X/src/modules/standard/mod_cern_meta.c
@@ -131,6 +131,7 @@
 #include <sys/stat.h>
 #include "util_script.h"
 #include "http_log.h"
+#include "http_request.h"
 
 #define DEFAULT_METADIR		".web"
 #define DEFAULT_METASUFFIX	".meta"
@@ -238,10 +239,10 @@ int add_cern_meta_data(request_rec *r)
     char *last_slash;
     char *real_file;
     char *scrap_book;
-    struct stat meta_stat;
     FILE *f;   
     cern_meta_config *cmc ;
     int rv;
+    request_rec *rr;
 
     cmc = get_module_config (r->server->module_config,
                            &cern_meta_module); 
@@ -276,30 +277,23 @@ int add_cern_meta_data(request_rec *r)
 
     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...
+    /* 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.
      */
-    if (stat(metafilename, &meta_stat) == -1) {
-	/* stat failed, possibly file missing */
+    rr = sub_req_lookup_file (metafilename, r);
+    if (rr->status != HTTP_OK) {
+	destroy_sub_req (rr);
 	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;
-    };
+    }
+    destroy_sub_req (rr);
 
     f = pfopen (r->pool, metafilename, "r");
-    
     if (f == NULL) {
+	if (errno == ENOENT) {
+	    return DECLINED;
+	}
         log_reason("meta file permissions deny server access", metafilename, r);
         return FORBIDDEN;
     };
diff --git a/APACHE_1_2_X/src/modules/standard/mod_dir.c b/APACHE_1_2_X/src/modules/standard/mod_dir.c
index bb803ec9f52..6ae62014abf 100644
--- a/APACHE_1_2_X/src/modules/standard/mod_dir.c
+++ b/APACHE_1_2_X/src/modules/standard/mod_dir.c
@@ -164,11 +164,17 @@ const char *add_ignore(cmd_parms *cmd, void *d, char *ext) {
 }
 
 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;
 }
@@ -408,7 +414,9 @@ int insert_readme(char *name, char *readme_fname, int rule, request_rec *r) {
     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) {
@@ -421,6 +429,14 @@ int insert_readme(char *name, char *readme_fname, int rule, request_rec *r) {
         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)
@@ -462,6 +478,9 @@ char *find_title(request_rec *r) {
     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;
diff --git a/unlabeled-1.26.2/src/modules/standard/mod_autoindex.c b/unlabeled-1.26.2/src/modules/standard/mod_autoindex.c
index bb803ec9f52..6ae62014abf 100644
--- a/unlabeled-1.26.2/src/modules/standard/mod_autoindex.c
+++ b/unlabeled-1.26.2/src/modules/standard/mod_autoindex.c
@@ -164,11 +164,17 @@ const char *add_ignore(cmd_parms *cmd, void *d, char *ext) {
 }
 
 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;
 }
@@ -408,7 +414,9 @@ int insert_readme(char *name, char *readme_fname, int rule, request_rec *r) {
     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) {
@@ -421,6 +429,14 @@ int insert_readme(char *name, char *readme_fname, int rule, request_rec *r) {
         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)
@@ -462,6 +478,9 @@ char *find_title(request_rec *r) {
     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;