buffer overflow under a particular rewrite configuration

[fan liu]

nginx conf：
location / {

rewrite ^{(/.*)$ $1?ip=$remote_addr&fetch=baidu? last;}

}

Surely, this config leads to url handling loop. I config in this way just in order to make the coredump appear easier.

The request url is:
/r/www/cache/g%E6%A0%A1%E8%BF%90%E5%8A%A8%E5%9C%BA%E4%B8%8A%E6%95%A3%E6%AD%A5%E3%80%82%E4%BB%96%E4%BB%8E%E6%A5%BC%E4%B8%8A%E8%B5%B0%E4%BA%86%E4%B8%8B%E6%9D%A5%EF%BC%8C%E7%9C%8B%E5%88%B0%E6%88%91%E7%A9%BF%E7%9A%84...www8090kkwww8090kk%E3%80%90%3Cem%3Ewww8090kk%3C/em%3E%E3%80%91_%3Cem%3Ewww8090kk%3C/em%3E%E2%80%BB%E9%AB%98%E6%B8%85%E2%80%BBwww8090kkwww8090kk%20=======...%3Cbr%3E%3Cspan%20class=

As you see, there are many '%', that's what leads to the coredump.

The coredump happens in this way:

When nginx handles rewrite declaration, it first estimates buffer length the destination needs. As to
this statement, it calls:
ngx_http_script_copy_capture_len_code
ngx_http_script_mark_args_code
ngx_http_script_copy_len_code
...

Then, nginx starts to translate the url, it calls:
ngx_http_script_copy_capture_code
ngx_http_script_copy_code
...

In function ngx_http_script_mark_args_code, e->is_args is set to 1.
As a result, when estimate dest buffer in function ngx_http_script_copy_capture_len_code,
ngx_escape_uri is not called, but when translate the url in function ngx_http_script_copy_capture_code, ngx_escape_uri is called.

That's the problem.

ngx_escape_uri makes '%' be THREE characters, while we only allocates ONE byte buffer space. So, coredump.

[Maxim Dounin]

Thank you for report. The following patch should fix this:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1336042504 -14400
# Node ID cdf28c793849c77b35e384a66bb8ce2755497401
# Parent  41c1eac9cc2468753f5cb1f1f0608ab09b11bb56
Rewrite: fixed escaping and possible segfault (ticket #162).

The following code resulted in incorrect escaping of uri and possible
segfault:

    location / {
        rewrite ^(.*) $1?c=$1;
        return 200 "$uri";
    }

If there were arguments in a rewrite's replacement string, and length was
actually calculated (due to duplicate captures as in the example above,
or variables present), the is_args flag was set and incorrectly copied
after length calculation.  This resulted in escaping applied to the uri part
of the replacement, resulting in incorrect escaping.  Additionally, buffer
was allocated without escaping expected, thus this also resulted in buffer
overrun and possible segfault.

diff --git a/src/http/ngx_http_script.c b/src/http/ngx_http_script.c
--- a/src/http/ngx_http_script.c
+++ b/src/http/ngx_http_script.c
@@ -1043,7 +1043,6 @@ ngx_http_script_regex_start_code(ngx_htt
         }
 
         e->buf.len = len;
-        e->is_args = le.is_args;
     }
 
     if (code->add_args && r->args.len) {

Tests added here: ​http://mdounin.ru/hg/nginx-tests/rev/7f5095965c88

[Maxim Dounin]

In [4618/nginx]:

Rewrite: fixed escaping and possible segfault (ticket #162).

The following code resulted in incorrect escaping of uri and possible
segfault:

location / {

rewrite ^{(.*) $1?c=$1;
return 200 "$uri";}

}

If there were arguments in a rewrite's replacement string, and length was
actually calculated (due to duplicate captures as in the example above,
or variables present), the is_args flag was set and incorrectly copied
after length calculation. This resulted in escaping applied to the uri part
of the replacement, resulting in incorrect escaping. Additionally, buffer
was allocated without escaping expected, thus this also resulted in buffer
overrun and possible segfault.

[Maxim Dounin]

Fix committed.

[Maxim Dounin]

In [4665/nginx]:

Merge of r4618: rewrite escaping fix (ticket #162).

The following code resulted in incorrect escaping of uri and possible
segfault:

location / {

rewrite ^{(.*) $1?c=$1;
return 200 "$uri";}

}

If there were arguments in a rewrite's replacement string, and length was
actually calculated (due to duplicate captures as in the example above,
or variables present), the is_args flag was set and incorrectly copied
after length calculation. This resulted in escaping applied to the uri part
of the replacement, resulting in incorrect escaping. Additionally, buffer
was allocated without escaping expected, thus this also resulted in buffer
overrun and possible segfault.