Re: X-Accel-Redirect и uri escape

Роман Шишнев rommer at active.by
Tue Nov 26 04:59:46 UTC 2013


Hello,

Хотел обойтись малой кровью=.
Но раз ssi и dav туда-же, то вот мой следующий опус.
Надеюсь, на этот раз в нужном направлении.

Заодно закроется бага с редиректом "../blablabla"
которая считалась safe.

P.S. это едиственный coding style?
http://wiki.nginx.org/CodingStyle

On 11/26/2013 02:35 AM, Maxim Dounin wrote:
> Hello!
>
> On Tue, Nov 26, 2013 at 01:47:36AM +0300, Роман Шишнев wrote:
>
>> Hello,
>>
>> Тогда всё проще и вот так.
>
> Совершенно точно нет.  Проверить, разескейпить, потом ещё раз
> проверить - это совсем не тот подход, которым следует
> пользоваться.  Не говоря уже о том, что ssi и dav это не лечит.
>
> Перечитайте ещё раз то, что уже было написано по данному вопросу.
> Тикет, всё-таки, не совсем бесполезен, там по ссылкам есть review
> предыдущих попыток.
>
> http://trac.nginx.org/nginx/ticket/316
>
>> P.S. зачем flags в ngx_http_parse_unsafe_uri()
>> передается по ссылке?
>
> Исходно этот параметр использовался для возврата флагов, в
> частности - NGX_HTTP_ZERO_IN_URI:
>
> http://hg.nginx.org/nginx/diff/58475592100c/src/http/ngx_http_parse.c
>
> С тех пор флаг NGX_HTTP_ZERO_IN_URI упразднили, но интерфейс
> остался.
>

-- 
С уважением,
Роман Шишнёв,
CTO | ActiveCloud | http://www.active.by
Т +375 17 2 911 511 доб. 308 | rommer at active.by
Облачные решения | Серверы и инфраструктура | IaaS | SaaS | Хостинг
-------------- next part --------------
diff -Nrup a/src/http/ngx_http_parse.c d/src/http/ngx_http_parse.c
--- a/src/http/ngx_http_parse.c 2013-11-19 15:25:25.000000000 +0400
+++ d/src/http/ngx_http_parse.c 2013-11-26 08:43:58.611996498 +0400
@@ -1777,57 +1777,191 @@ ngx_int_t
 ngx_http_parse_unsafe_uri(ngx_http_request_t *r, ngx_str_t *uri,
     ngx_str_t *args, ngx_uint_t *flags)
 {
-    u_char  ch, *p;
-    size_t  len;
+    ngx_str_t original_uri;
+    u_char   *d, *s, ch, c, decoded;
+    size_t    len;
+    enum {
+        sw_usual = 0,
+        sw_quoted,
+        sw_quoted_second
+    } state;
 
+    s = uri->data;
+    d = uri->data;
     len = uri->len;
-    p = uri->data;
 
-    if (len == 0 || p[0] == '?') {
+    state = 0;
+    decoded = 0;
+
+    if (len == 0 || s[0] == '?') {
+        if (*flags & NGX_HTTP_LOG_UNSAFE) {
+            original_uri.len = uri->len;
+            original_uri.data = uri->data;
+        }
+
         goto unsafe;
     }
 
-    if (p[0] == '.' && len == 3 && p[1] == '.' && (ngx_path_separator(p[2]))) {
-        goto unsafe;
+    if (*flags & NGX_HTTP_LOG_UNSAFE) {
+        original_uri.len = uri->len;
+        original_uri.data = ngx_pstrdup(r->pool, uri);
     }
 
-    for ( /* void */ ; len; len--) {
+    while (len--) {
 
-        ch = *p++;
+        ch = *s++;
 
-        if (usual[ch >> 5] & (1 << (ch & 0x1f))) {
-            continue;
-        }
+        switch (state) {
+        case sw_usual:
+            if (usual[ch >> 5] & (1 << (ch & 0x1f))) {
+                *d++ = ch;
+                break;
+            }
 
-        if (ch == '?') {
-            args->len = len - 1;
-            args->data = p;
-            uri->len -= len;
+            if (ch == '\0') {
+                goto unsafe;
+            }
 
-            return NGX_OK;
-        }
+            if (ch == '%') {
+                state = sw_quoted;
+                break;
+            }
 
-        if (ch == '\0') {
-            goto unsafe;
-        }
+            if (ch == '?') {
+                goto args;
+            }
 
-        if (ngx_path_separator(ch) && len > 2) {
+            if (ngx_path_separator(ch)) {
 
-            /* detect "/../" */
+                if (d - 2 == uri->data &&
+                    *(d - 2) == '.' && *(d - 1) == '.') {
+                    goto unsafe;
+                }
+
+                if (d - 2 > uri->data &&
+                    ngx_path_separator(*(d - 3)) &&
+                    *(d - 2) == '.' && *(d - 1) == '.') {
+                    goto unsafe;
+                }
+            }
+
+            *d++ = ch;
+            break;
+
+        case sw_quoted:
+            if (ch >= '0' && ch <= '9') {
+                decoded = (u_char) (ch - '0');
+                state = sw_quoted_second;
+                break;
+            }
 
-            if (p[0] == '.' && p[1] == '.' && ngx_path_separator(p[2])) {
+            c = (u_char) (ch | 0x20);
+            if (c >= 'a' && c <= 'f') {
+                decoded = (u_char) (c - 'a' + 10);
+                state = sw_quoted_second;
+                break;
+            }
+
+            if (ch == '\0') {
                 goto unsafe;
             }
+
+            if (ch == '%') {
+                *d++ = '%';
+                break;
+            }
+
+            if (ch == '?') {
+                *d++ = '%';
+                goto args;
+            }
+
+            *d++ = '%'; *d++ = ch;
+            state = sw_usual;
+            break;
+
+        case sw_quoted_second:
+
+            state = sw_usual;
+
+            if (ch >= '0' && ch <= '9') {
+                ch = (u_char) ((decoded << 4) + ch - '0');
+                *d++ = ch;
+                break;
+            }
+
+            c = (u_char) (ch | 0x20);
+            if (c >= 'a' && c <= 'f') {
+                ch = (u_char) ((decoded << 4) + c - 'a' + 10);
+
+                if (ngx_path_separator(ch)) {
+
+                    if (d - 2 == uri->data &&
+                        *(d - 2) == '.' && *(d - 1) == '.') {
+                        goto unsafe;
+                    }
+
+                    if (d - 2 > uri->data &&
+                        ngx_path_separator(*(d - 3)) &&
+                        *(d - 2) == '.' && *(d - 1) == '.') {
+                        goto unsafe;
+                    }
+                }
+
+                *d++ = ch;
+                break;
+            }
+
+            if (ch == '\0') {
+                goto unsafe;
+            }
+
+            if (ch == '%') {
+                *d++ = '%'; *d++ = *(s - 2);
+                state = sw_quoted;
+                break;
+            }
+
+            if (ch == '?') {
+                *d++ = '%'; *d++ = *(s - 2);
+                goto args;
+            }
+
+            *d++ = '%'; *d++ = *(s - 2); *d++ = ch;
+            break;
         }
     }
 
+    switch (state) {
+    case sw_usual:
+        break;
+    case sw_quoted:
+        *d++ = '%';
+        break;
+    case sw_quoted_second:
+        *d++ = '%';
+        *d++ = *(s - 1);
+        break;
+    }
+
+    uri->len = d - uri->data;
+
+    return NGX_OK;
+
+args:
+
+    args->len = len;
+    args->data = s;
+
+    uri->len = d - uri->data;
+
     return NGX_OK;
 
 unsafe:
 
     if (*flags & NGX_HTTP_LOG_UNSAFE) {
         ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
-                      "unsafe URI \"%V\" was detected", uri);
+                      "unsafe URI \"%V\" was detected", &original_uri);
     }
 
     return NGX_ERROR;


Подробная информация о списке рассылки nginx-ru