[nginx] svn commit: r4679 - in branches/stable-1.2: . src/http src/os/win32

mdounin at mdounin.ru mdounin at mdounin.ru
Tue Jun 5 13:52:37 UTC 2012


Author: mdounin
Date: 2012-06-05 13:52:37 +0000 (Tue, 05 Jun 2012)
New Revision: 4679
URL: http://trac.nginx.org/nginx/changeset/4679/nginx

Log:
Merge of r4674, r4675, r4676: win32 fixes.

*) Win32: disallowed access to various non-canonical name variants.

   This includes trailings dots and spaces, NTFS streams (and short names, as
   previously checked).  The checks are now also done in ngx_file_info(), thus
   allowing to use the "try_files" directive to protect external scripts.

*) Win32: normalization of trailing dot inside uri.

   Windows treats "/directory./" identical to "/directory/".  Do the same
   when working on Windows.  Note that the behaviour is different from one
   with last path component (where multiple spaces and dots are ignored by
   Windows).

*) Win32: uris with ":$" are now rejected.

   There are too many problems with special NTFS streams, notably "::$data",
   "::$index_allocation" and ":$i30:$index_allocation".

   For now we don't reject all URIs with ":" like Apache does as there are no
   good reasons seen yet, and there are multiple programs using it in URLs
   (e.g. MediaWiki).


Modified:
   branches/stable-1.2/
   branches/stable-1.2/src/http/ngx_http_parse.c
   branches/stable-1.2/src/http/ngx_http_request.c
   branches/stable-1.2/src/os/win32/ngx_files.c

Index: branches/stable-1.2
===================================================================
--- branches/stable-1.2	2012-06-05 13:47:50 UTC (rev 4678)
+++ branches/stable-1.2	2012-06-05 13:52:37 UTC (rev 4679)

Property changes on: branches/stable-1.2
___________________________________________________________________
Modified: svn:mergeinfo
## -1 +1 ##
-/trunk:4611-4632,4641
+/trunk:4611-4632,4641,4674-4676
\ No newline at end of property
Modified: branches/stable-1.2/src/http/ngx_http_parse.c
===================================================================
--- branches/stable-1.2/src/http/ngx_http_parse.c	2012-06-05 13:47:50 UTC (rev 4678)
+++ branches/stable-1.2/src/http/ngx_http_parse.c	2012-06-05 13:52:37 UTC (rev 4679)
@@ -543,6 +543,13 @@
 
             switch (ch) {
             case '/':
+#if (NGX_WIN32)
+                if (r->uri_ext == p) {
+                    r->complex_uri = 1;
+                    state = sw_uri;
+                    break;
+                }
+#endif
                 r->uri_ext = NULL;
                 state = sw_after_slash_in_uri;
                 break;
@@ -1117,6 +1124,12 @@
             switch(ch) {
 #if (NGX_WIN32)
             case '\\':
+                if (u - 2 >= r->uri.data
+                    && *(u - 1) == '.' && *(u - 2) != '.')
+                {
+                    u--;
+                }
+
                 r->uri_ext = NULL;
 
                 if (p == r->uri_start + r->uri.len) {
@@ -1134,6 +1147,13 @@
                 break;
 #endif
             case '/':
+#if (NGX_WIN32)
+                if (u - 2 >= r->uri.data
+                    && *(u - 1) == '.' && *(u - 2) != '.')
+                {
+                    u--;
+                }
+#endif
                 r->uri_ext = NULL;
                 state = sw_slash;
                 *u++ = ch;

Modified: branches/stable-1.2/src/http/ngx_http_request.c
===================================================================
--- branches/stable-1.2/src/http/ngx_http_request.c	2012-06-05 13:47:50 UTC (rev 4678)
+++ branches/stable-1.2/src/http/ngx_http_request.c	2012-06-05 13:52:37 UTC (rev 4679)
@@ -812,8 +812,29 @@
 
 #if (NGX_WIN32)
             {
-            u_char  *p;
+            u_char  *p, *last;
 
+            p = r->uri.data;
+            last = r->uri.data + r->uri.len;
+
+            while (p < last) {
+
+                if (*p++ == ':') {
+
+                    /*
+                     * this check covers "::$data", "::$index_allocation" and
+                     * ":$i30:$index_allocation"
+                     */
+
+                    if (p < last && *p == '$') {
+                        ngx_log_error(NGX_LOG_INFO, c->log, 0,
+                                      "client sent unsafe win32 URI");
+                        ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
+                        return;
+                    }
+                }
+            }
+
             p = r->uri.data + r->uri.len - 1;
 
             while (p > r->uri.data) {
@@ -828,11 +849,6 @@
                     continue;
                 }
 
-                if (ngx_strncasecmp(p - 6, (u_char *) "::$data", 7) == 0) {
-                    p -= 7;
-                    continue;
-                }
-
                 break;
             }
 

Modified: branches/stable-1.2/src/os/win32/ngx_files.c
===================================================================
--- branches/stable-1.2/src/os/win32/ngx_files.c	2012-06-05 13:47:50 UTC (rev 4678)
+++ branches/stable-1.2/src/os/win32/ngx_files.c	2012-06-05 13:52:37 UTC (rev 4679)
@@ -11,6 +11,8 @@
 
 #define NGX_UTF16_BUFLEN  256
 
+static ngx_int_t ngx_win32_check_filename(u_char *name, u_short *u,
+    size_t len);
 static u_short *ngx_utf8_to_utf16(u_short *utf16, u_char *utf8, size_t *len);
 
 
@@ -20,8 +22,7 @@
 ngx_open_file(u_char *name, u_long mode, u_long create, u_long access)
 {
     size_t      len;
-    u_long      n;
-    u_short    *u, *lu;
+    u_short    *u;
     ngx_fd_t    fd;
     ngx_err_t   err;
     u_short     utf16[NGX_UTF16_BUFLEN];
@@ -34,25 +35,11 @@
     }
 
     fd = INVALID_HANDLE_VALUE;
-    lu = NULL;
 
-    if (create == NGX_FILE_OPEN) {
-
-        lu = malloc(len * 2);
-        if (lu == NULL) {
-            goto failed;
-        }
-
-        n = GetLongPathNameW(u, lu, len);
-
-        if (n == 0) {
-            goto failed;
-        }
-
-        if (n != len - 1 || _wcsicmp(u, lu) != 0) {
-            ngx_set_errno(NGX_ENOENT);
-            goto failed;
-        }
+    if (create == NGX_FILE_OPEN
+        && ngx_win32_check_filename(name, u, len) != NGX_OK)
+    {
+        goto failed;
     }
 
     fd = CreateFileW(u, mode,
@@ -61,18 +48,12 @@
 
 failed:
 
-    err = ngx_errno;
-
-    if (lu) {
-        ngx_free(lu);
-    }
-
     if (u != utf16) {
+        err = ngx_errno;
         ngx_free(u);
+        ngx_set_errno(err);
     }
 
-    ngx_set_errno(err);
-
     return fd;
 }
 
@@ -294,14 +275,14 @@
         return NGX_FILE_ERROR;
     }
 
-    rc = GetFileAttributesExW(u, GetFileExInfoStandard, &fa);
+    rc = NGX_FILE_ERROR;
 
-    if (u != utf16) {
-        err = ngx_errno;
-        ngx_free(u);
-        ngx_set_errno(err);
+    if (ngx_win32_check_filename(file, u, len) != NGX_OK) {
+        goto failed;
     }
 
+    rc = GetFileAttributesExW(u, GetFileExInfoStandard, &fa);
+
     sb->dwFileAttributes = fa.dwFileAttributes;
     sb->ftCreationTime = fa.ftCreationTime;
     sb->ftLastAccessTime = fa.ftLastAccessTime;
@@ -309,6 +290,14 @@
     sb->nFileSizeHigh = fa.nFileSizeHigh;
     sb->nFileSizeLow = fa.nFileSizeLow;
 
+failed:
+
+    if (u != utf16) {
+        err = ngx_errno;
+        ngx_free(u);
+        ngx_set_errno(err);
+    }
+
     return rc;
 }
 
@@ -640,6 +629,148 @@
 }
 
 
+static ngx_int_t
+ngx_win32_check_filename(u_char *name, u_short *u, size_t len)
+{
+    u_char     *p, ch;
+    u_long      n;
+    u_short    *lu;
+    ngx_err_t   err;
+    enum {
+        sw_start = 0,
+        sw_normal,
+        sw_after_slash,
+        sw_after_colon,
+        sw_after_dot
+    } state;
+
+    /* check for NTFS streams (":"), trailing dots and spaces */
+
+    lu = NULL;
+    state = sw_start;
+
+    for (p = name; *p; p++) {
+        ch = *p;
+
+        switch (state) {
+
+        case sw_start:
+
+            /*
+             * skip till first "/" to allow paths starting with drive and
+             * relative path, like "c:html/"
+             */
+
+            if (ch == '/' || ch == '\\') {
+                state = sw_after_slash;
+            }
+
+            break;
+
+        case sw_normal:
+
+            if (ch == ':') {
+                state = sw_after_colon;
+                break;
+            }
+
+            if (ch == '.' || ch == ' ') {
+                state = sw_after_dot;
+                break;
+            }
+
+            if (ch == '/' || ch == '\\') {
+                state = sw_after_slash;
+                break;
+            }
+
+            break;
+
+        case sw_after_slash:
+
+            if (ch == '/' || ch == '\\') {
+                break;
+            }
+
+            if (ch == '.') {
+                break;
+            }
+
+            if (ch == ':') {
+                state = sw_after_colon;
+                break;
+            }
+
+            state = sw_normal;
+            break;
+
+        case sw_after_colon:
+
+            if (ch == '/' || ch == '\\') {
+                state = sw_after_slash;
+                break;
+            }
+
+            goto invalid;
+
+        case sw_after_dot:
+
+            if (ch == '/' || ch == '\\') {
+                goto invalid;
+            }
+
+            if (ch == ':') {
+                goto invalid;
+            }
+       
+            if (ch == '.' || ch == ' ') {
+                break;
+            }
+
+            state = sw_normal;
+            break;
+        }
+    }
+
+    if (state == sw_after_dot) {
+        goto invalid;
+    }
+
+    /* check if long name match */
+
+    lu = malloc(len * 2);
+    if (lu == NULL) {
+        return NGX_ERROR;
+    }
+
+    n = GetLongPathNameW(u, lu, len);
+
+    if (n == 0) {
+        goto failed;
+    }
+
+    if (n != len - 1 || _wcsicmp(u, lu) != 0) {
+        goto invalid;
+    }
+
+    return NGX_OK;
+
+invalid:
+
+    ngx_set_errno(NGX_ENOENT);
+
+failed:
+
+    if (lu) {
+        err = ngx_errno;
+        ngx_free(lu);
+        ngx_set_errno(err);
+    }
+
+    return NGX_ERROR;
+}
+
+
 static u_short *
 ngx_utf8_to_utf16(u_short *utf16, u_char *utf8, size_t *len)
 {



More information about the nginx-devel mailing list