httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Sutton <p...@c2.net>
Subject [PATCH] Win32 device files
Date Wed, 02 Dec 1998 16:43:11 GMT
Below is a patch to fix a security problem when accessing Windows 32
device files. The problem is if a requrest is made for a URI such as /aux.
This is mapped onto a special DOS device file called AUX (no matter which
directory is the document root). This can cause problems. There are other
DOS device files as well, which at worse can cause a lock-up of the
server.

The patch adds a new OS abstraction: ap_os_is_filename_valid(). This is
given a path and returns 0 or 1 depending on whether the path name looks
valid or not, according to various rules.

This function is called from three places: 

  First, whenever we are going to open a configuration file with
  pcfg_openfile. This guards against references to configuration
  files with invalid filenames.

  Secondly, in get_path_info when trying to decide how much of a request
  URI is the real path and how much is the PATH_INFO. This is to avoid
  stat()'ing an invalid filename. (Note that get_path_info() works
  backwards from the complete path, so we cannot reject invalid filenames
  at this stage because the invalid part may be in the PATH_INFO).

  Thirdly, after the real filename has been found in directory_walk()
  to avoid trying to actually open an invalid filename when returning
  a response. We return HTTP_FORBIDDEN if we have an invalid filename
  at this point.

This patch should help fix the security hole, but has a few drawbacks: it
is slow, it does not cover all situations when we open files (e.g. you can
still specify PidFile /LPT1), and it will reject use of NUL in
AccessConfig or ResourceConfig directives. It will also reject invalid
path components which may be on a remote share where the path component is
infact valid.

Paul
--
Paul Sutton, C2Net Europe                    http://www.eu.c2.net/~paul/
Editor, Apache Week .. the latest Apache news http://www.apacheweek.com/

diff -ru -x CVS -x *.dsp -x *.mak ../apache-1.3/src/include/httpd.h ./include/httpd.h
--- ../apache-1.3/src/include/httpd.h	Fri Nov 13 14:43:53 1998
+++ ./include/httpd.h	Sat Nov 28 13:34:11 1998
@@ -1018,7 +1018,8 @@
 API_EXPORT(char *) ap_os_canonical_filename(pool *p, const char *file);
 #ifdef WIN32
 API_EXPORT(char *) ap_os_case_canonical_filename(pool *pPool, const char *szFile);
-API_EXPORT(char *) ap_os_systemcase_filename(pool *pPool, const char *szFile);
+API_EXPORT(char *) ap_os_systemcase_filename(pool *pPool, const char *szFile);
+API_EXPORT(int) ap_os_is_filename_valid(const char *file);
 #else
 #define ap_os_case_canonical_filename(p,f) ap_os_canonical_filename(p,f)
 #define ap_os_systemcase_filename(p,f) ap_os_canonical_filename(p,f)
diff -ru -x CVS -x *.dsp -x *.mak ../apache-1.3/src/main/http_request.c ./main/http_request.c
--- ../apache-1.3/src/main/http_request.c	Sat Nov 28 14:33:09 1998
+++ ./main/http_request.c	Wed Dec  2 16:27:05 1998
@@ -234,9 +234,27 @@
 
         *cp = '\0';
 
+#ifdef WIN32
+	/*
+	 * We must not stat() filenames such as "/file/aux" since it can cause
+	 * delays or lockups. So pretend that they do not exist by returning
+	 * an ENOENT error. This will force us to drop that part of the path and
+	 * keep looking back for a "real" file that exists, while still allowing
+	 * the "invalid" path parts within the PATH_INFO.
+	 */
+	if (!ap_os_is_filename_valid(path)) {
+	    errno = ENOENT;
+	    rv = -1;
+        } else {
+#endif
+
         errno = 0;
         rv = stat(path, &r->finfo);
 
+#ifdef WIN32
+	}
+#endif
+
         if (cp != end)
             *cp = '/';
 
@@ -395,6 +413,14 @@
     ap_no2slash(test_filename);
     num_dirs = ap_count_dirs(test_filename);
 
+#ifdef WIN32
+    if (!ap_os_is_filename_valid(r->filename)) {
+        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
+		      "Filename is not valid: %s", r->filename);
+        return HTTP_FORBIDDEN;
+    }
+#endif
+
     if ((res = check_safe_file(r))) {
         return res;
     }
@@ -804,7 +830,7 @@
         rnew->uri = ap_make_full_path(rnew->pool, udir, new_file);
         rnew->filename = ap_make_full_path(rnew->pool, fdir, new_file);
 	ap_parse_uri(rnew, rnew->uri);    /* fill in parsed_uri values */
-        if (stat(rnew->filename, &rnew->finfo) < 0) {
+    	if (stat(rnew->filename, &rnew->finfo) < 0) {
             rnew->finfo.st_mode = 0;
         }
 
diff -ru -x CVS -x *.dsp -x *.mak ../apache-1.3/src/main/util.c ./main/util.c
--- ../apache-1.3/src/main/util.c	Sat Nov 28 14:33:15 1998
+++ ./main/util.c	Sun Nov 29 12:31:42 1998
@@ -761,6 +761,16 @@
         return NULL;
     }
 
+#if defined(WIN32)
+    if (!ap_os_is_filename_valid(name)) {
+        ap_log_error(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, NULL,
+                    "Access to config file %s denied: not a valid filename",
+                    name);
+	errno = EACCES;
+        return NULL;
+    }
+#endif
+
     file = ap_pfopen(p, name, "r");
 #ifdef DEBUG
     saved_errno = errno;
diff -ru -x CVS -x *.dsp -x *.mak ../apache-1.3/src/os/win32/util_win32.c ./os/win32/util_win32.c
--- ../apache-1.3/src/os/win32/util_win32.c	Fri Nov 13 14:44:11 1998
+++ ./os/win32/util_win32.c	Wed Dec  2 16:27:14 1998
@@ -293,6 +293,7 @@
         return -1;
     }
 
+  
     if (szPath[0] == '/' && szPath[1] == '/') {
 	char buf[_MAX_PATH];
 	char *s;
@@ -533,4 +534,145 @@
         return_value = strftime(s, max, new_format, tm);
     }
     return return_value;
+}
+
+/*
+ * ap_os_is_filename_valid is given a filename, and returns 0 if the filename
+ * is not valid for use on this system. On Windows, this means it fails any
+ * of the tests below. Otherwise returns 1.
+ *
+ * Test for filename validity on Win32. This is of tests come in part from
+ * the MSDN article at "Technical Articles, Windows Platform, Base Services,
+ * Guidelines, Making Room for Long Filenames" although the information
+ * in MSDN about filename testing is incomplete or conflicting. There is a
+ * similar set of tests in "Technical Articles, Windows Platform, Base Services,
+ * Guidelines, Moving Unix Applications to Windows NT".
+ *
+ * The tests are:
+ *
+ * 1) total path length greater than MAX_PATH
+ *
+ * 2) anything using the octets 0-31 or characters " < > | :
+ *    (these are reserved for Windows use in filenames. In addition
+ *     each file system has its own additional characters that are
+ *     invalid. See KB article Q100108 for more details).
+ *
+ * 3) anything ending in "." (no matter how many)
+ *    (filename doc, doc. and doc... all refer to the same file)
+ *
+ * 4) any segment in which the basename (before first period) matches
+ *    one of the DOS device names
+ *    (the list comes from KB article Q100108 although some people
+ *     reports that additional names such as "COM5" are also special
+ *     devices).
+ *
+ * If the path fails ANY of these tests, the result must be to deny access.
+ */
+
+API_EXPORT(int) ap_os_is_filename_valid(const char *file)
+{
+    const char *segstart;
+    char seglength;
+    const char *pos;
+    static const char * const invalid_characters = "?\"<>*|:";
+    static const char * const invalid_filenames[] = { 
+	"CON", "AUX", "COM1", "COM2", "COM3", 
+	"COM4", "LPT1", "LPT2", "LPT3", "PRN", "NUL", NULL 
+    };
+
+    /* Test 1 */
+    if (strlen(file) > MAX_PATH) {
+	/* Path too long for Windows. Note that this test is not valid
+	 * if the path starts with //?/ or \\?\. */
+	return 0;
+    }
+
+    pos = file;
+
+    /* Skip any leading non-path components. This can be either a
+     * drive letter such as C:, or a UNC path such as \\SERVER\SHARE\.
+     * We continue and check the rest of the path based on the rules above.
+     * This means we could eliminate valid filenames from servers which
+     * are not running NT (such as Samba).
+     */
+
+    if (pos[0] && pos[1] == ':') {
+	/* Skip leading drive letter */
+	pos += 2;
+    } else {
+	if ((pos[0] == '\\' || pos[0] == '/') &&
+	    (pos[1] == '\\' || pos[1] == '/')) {
+	    /* Is a UNC, so skip the server name and share name */
+	    pos += 2;
+	    while (*pos && *pos != '/' && *pos != '\\')
+		pos++;
+	    if (!*pos) {
+		/* No share name */
+		return 0;
+	    }
+	    pos++;	/* Move to start of share name */
+	    while (*pos && *pos != '/' && *pos != '\\')
+		pos++;
+	    if (!*pos) {
+		/* No path information */
+		return 0;
+	    }
+	}
+    }
+
+    while (*pos) {
+	int idx;
+	int baselength;
+
+	while (*pos == '/' || *pos == '\\') {
+    	    pos++;
+	}
+	if (*pos == '\0') {
+	    break;
+	}
+	segstart = pos;	/* start of segment */
+	while (*pos && *pos != '/' && *pos != '\\') {
+	    pos++;
+	}
+	seglength = pos - segstart;
+	/* 
+	 * Now we have a segment of the path, starting at position "segstart"
+	 * and length "seglength"
+	 */
+
+	/* Test 2 */
+	for (idx = 0; idx < seglength; idx++) {
+	    if (segstart[idx] < 32 ||
+		strchr(invalid_characters, segstart[idx])) {
+		return 0;
+	    }
+	}
+
+	/* Test 3 */
+	if (segstart[seglength-1] == '.') {
+	    return 0;
+	}
+
+	/* Test 4 */
+	for (baselength = 0; baselength < seglength; baselength++) {
+	    if (segstart[baselength] == '.') {
+		break;
+	    }
+	}
+
+	/* baselength is the number of characters in the base path of
+	 * the segment (which could be the same as the whole segment length,
+	 * if it does not include any dot characters). */
+	if (baselength == 3 || baselength == 4) {
+	    for (idx = 0; invalid_filenames[idx]; idx++) {
+		if (!strnicmp(invalid_filenames[idx], segstart, baselength)) {
+		    return 0;
+		}
+	    }
+	}
+    }
+
+    return 1;
 }


Mime
View raw message