You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Paul Sutton <pa...@c2.net> on 1998/12/02 17:43:11 UTC
[PATCH] Win32 device files
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;
}
RE: [PATCH] Win32 device files
Posted by Ken Parzygnat <kp...@raleigh.ibm.com>.
>
> 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.
>
Cool. I'm really glad you created the new function instead of putting this
logic in the os_canonical_filename stuff!! It should be easier to manage.
I've played around with the patch and things are looking good, but I did find
two things:
If I create an alias like:
Alias /croot/ c:/
The patch will make this fail, while the pre-patched code works fine. I suspect
the culprit is test #3. I seem to remember a place or two in the code that will
append a '.' to names like x:/
I wonder if we need test #3 in ap_os_is_filename_valid since
ap_os_case_canonical_filename silently removes the trailing '.'?
Also, an alias like:
Alias /sharedir/ //machine/share/
Fails with the patch. It seems that there may be a problem
with the .htaccess processing, though I haven't tracked that down.
- - - - - - - - - - - - - - - - - -
Ken Parzygnat
email: kparz@raleigh.ibm.com