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

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel