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@active.by
Облачные решения | Серверы и инфраструктура | IaaS | SaaS | Хостинг
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 mailing list
nginx-ru@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-ru