[PATCH v2] Make ngx_http_parse_unsafe_uri() to be able to unescape uri

Raman Shishniou rommer at activecloud.com
Wed Dec 18 03:09:44 UTC 2013


Hello,

Is something wrong with this patch?

On 12/11/2013 08:48 AM, Raman Shishniou wrote:
> # HG changeset patch
> # User Raman Shishniou <rommer at activecloud.com>
> # Date 1386740673 -10800
> # Node ID 353840924c0ffd1203c8d0894f2128a2548f80c5
> # Parent  a279d2a33dbfbad511f4415f833c35a60e46bb76
> Make ngx_http_parse_unsafe_uri() to be able to unescape uri
> 
> It makes possible to use an escaped uri with "X-Accel-Redirect" header in
> http_upstream module and "Destination" header http_dav module. No need
> to unescape uri in http_ssi_filter module any more.
> 
> diff --git a/src/http/modules/ngx_http_ssi_filter_module.c b/src/http/modules/ngx_http_ssi_filter_module.c
> --- a/src/http/modules/ngx_http_ssi_filter_module.c
> +++ b/src/http/modules/ngx_http_ssi_filter_module.c
> @@ -1982,8 +1982,6 @@
>  ngx_http_ssi_include(ngx_http_request_t *r, ngx_http_ssi_ctx_t *ctx,
>      ngx_str_t **params)
>  {
> -    u_char                      *dst, *src;
> -    size_t                       len;
>      ngx_int_t                    rc, key;
>      ngx_str_t                   *uri, *file, *wait, *set, *stub, args;
>      ngx_buf_t                   *b;
> @@ -2054,18 +2052,6 @@
>          return rc;
>      }
>  
> -    dst = uri->data;
> -    src = uri->data;
> -
> -    ngx_unescape_uri(&dst, &src, uri->len, NGX_UNESCAPE_URI);
> -
> -    len = (uri->data + uri->len) - src;
> -    if (len) {
> -        dst = ngx_movemem(dst, src, len);
> -    }
> -
> -    uri->len = dst - uri->data;
> -
>      ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
>                     "ssi include: \"%V\"", uri);
>  
> diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
> --- a/src/http/ngx_http_parse.c
> +++ b/src/http/ngx_http_parse.c
> @@ -9,6 +9,8 @@
>  #include <ngx_core.h>
>  #include <ngx_http.h>
>  
> +static ngx_inline ngx_int_t
> +ngx_http_parse_test_doubledot(const u_char *p, const u_char *begin);
>  
>  static uint32_t  usual[] = {
>      0xffffdbfe, /* 1111 1111 1111 1111  1101 1011 1111 1110 */
> @@ -1776,35 +1778,75 @@
>  }
>  
>  
> +static ngx_inline ngx_int_t
> +ngx_http_parse_test_doubledot(const u_char *p, const u_char *begin)
> +{
> +    /* assume *p is the path separator or
> +       p points to the next byte after the end */
> +
> +    if (p - 2 > begin &&
> +        *(p - 1) == '.' && *(p - 2) == '.' &&
> +        ngx_path_separator(*(p - 3))) {
> +        return 1;
> +    }
> +
> +    if (p - 2 == begin &&
> +        begin[0] == '.' && begin[1] == '.') {
> +        return 1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>  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;
> -
> +    u_char   *src, *dst, *newuri, ch, c, decoded;
> +    ngx_int_t unescape;
> +    size_t    len;
> +    enum {
> +        sw_usual = 0,
> +        sw_quoted,
> +        sw_quoted_second
> +    } state;
> +
> +    src = uri->data;
>      len = uri->len;
> -    p = uri->data;
> -
> -    if (len == 0 || p[0] == '?') {
> +    unescape = 0;
> +
> +    if (len == 0 || src[0] == '?') {
>          goto unsafe;
>      }
>  
> -    if (p[0] == '.' && len == 3 && p[1] == '.' && (ngx_path_separator(p[2]))) {
> -        goto unsafe;
> -    }
> -
>      for ( /* void */ ; len; len--) {
>  
> -        ch = *p++;
> +        ch = *src++;
>  
>          if (usual[ch >> 5] & (1 << (ch & 0x1f))) {
>              continue;
>          }
>  
> +        if (ch == '%') {
> +            unescape++;
> +            continue;
> +        }
> +
>          if (ch == '?') {
>              args->len = len - 1;
> -            args->data = p;
> +            args->data = src;
> +
> +            if (unescape) {
> +                src--;
> +                goto unescape;
> +            }
> +
> +            /* detect "/.." at the end or whole uri is ".." */
> +            if (ngx_http_parse_test_doubledot(src - 1, uri->data)) {
> +                goto unsafe;
> +            }
> +
>              uri->len -= len;
>  
>              return NGX_OK;
> @@ -1814,16 +1856,151 @@
>              goto unsafe;
>          }
>  
> -        if (ngx_path_separator(ch) && len > 2) {
> -
> -            /* detect "/../" */
> -
> -            if (p[0] == '.' && p[1] == '.' && ngx_path_separator(p[2])) {
> +        /* detect "../" at the beginning or "/../" in the middle */
> +        if (ngx_path_separator(ch) &&
> +            ngx_http_parse_test_doubledot(src - 1, uri->data)) {
> +            goto unsafe;
> +        }
> +    }
> +
> +    /* detect "/.." at the end or whole uri is ".." */
> +    if (ngx_http_parse_test_doubledot(src, uri->data)) {
> +        goto unsafe;
> +    }
> +
> +    if (!unescape) {
> +        return NGX_OK;
> +    }
> +
> +unescape:
> +
> +    len = src - uri->data;
> +    newuri = ngx_pnalloc(r->pool, len);
> +
> +    if (newuri == NULL) {
> +        return NGX_ERROR;
> +    }
> +
> +    ngx_memcpy(newuri, uri->data, len);
> +
> +    src = uri->data;
> +    dst = newuri;
> +    decoded = 0;
> +    state = 0;
> +
> +    for ( /* void */ ; len; len--) {
> +
> +        ch = *src++;
> +
> +        switch (state) {
> +
> +        case sw_usual:
> +            if (usual[ch >> 5] & (1 << (ch & 0x1f))) {
> +                *dst++ = ch;
> +                break;
> +            }
> +
> +            if (ch == '%') {
> +                state = sw_quoted;
> +                break;
> +            }
> +
> +            /* detect "../" at the beginning or "/../" in the middle */
> +            if (ngx_path_separator(ch) &&
> +                ngx_http_parse_test_doubledot(dst, newuri)) {
>                  goto unsafe;
>              }
> +
> +            *dst++ = ch;
> +            break;
> +
> +        case sw_quoted:
> +            if (ch >= '0' && ch <= '9') {
> +                decoded = (u_char) (ch - '0');
> +                state = sw_quoted_second;
> +                break;
> +            }
> +
> +            c = (u_char) (ch | 0x20);
> +            if (c >= 'a' && c <= 'f') {
> +                decoded = (u_char) (c - 'a' + 10);
> +                state = sw_quoted_second;
> +                break;
> +            }
> +
> +            if (ch == '%') {
> +                *dst++ = '%';
> +                break;
> +            }
> +
> +            *dst++ = '%';
> +            *dst++ = ch;
> +            state = sw_usual;
> +            break;
> +
> +        case sw_quoted_second:
> +
> +            state = sw_usual;
> +
> +            if (ch >= '0' && ch <= '9') {
> +                ch = (u_char) ((decoded << 4) + ch - '0');
> +
> +                if (ch == '\0') {
> +                    goto unsafe;
> +                }
> +
> +                *dst++ = ch;
> +                break;
> +            }
> +
> +            c = (u_char) (ch | 0x20);
> +            if (c >= 'a' && c <= 'f') {
> +                ch = (u_char) ((decoded << 4) + c - 'a' + 10);
> +
> +                /* detect "../" at the beginning or "/../" in the middle */
> +                if (ngx_path_separator(ch) &&
> +                    ngx_http_parse_test_doubledot(dst, newuri)) {
> +                    goto unsafe;
> +                }
> +
> +                *dst++ = ch;
> +                break;
> +            }
> +
> +            if (ch == '%') {
> +                *dst++ = '%';
> +                *dst++ = *(src - 2);
> +                state = sw_quoted;
> +                break;
> +            }
> +
> +            *dst++ = '%';
> +            *dst++ = *(src - 2);
> +            *dst++ = ch;
> +            break;
>          }
>      }
>  
> +    switch (state) {
> +    case sw_usual:
> +        break;
> +    case sw_quoted:
> +        *dst++ = '%';
> +        break;
> +    case sw_quoted_second:
> +        *dst++ = '%';
> +        *dst++ = *(src - 1);
> +        break;
> +    }
> +
> +    /* detect "/.." at the end or whole uri is ".." */
> +    if (ngx_http_parse_test_doubledot(dst, newuri)) {
> +        goto unsafe;
> +    }
> +
> +    uri->len = dst - newuri;
> +    uri->data = newuri;
> +
>      return NGX_OK;
>  
>  unsafe:
> 
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
> 



More information about the nginx-devel mailing list