Hello!
On Sun, Aug 16, 2015 at 05:28:34PM +0300, Markus Linnala wrote:
> Sometimes nginx accepts invalid configuration.
>
> Like:
>
> <>
> worker_processes 1;
>
> events {
> worker_connections 1024;
> }
>
> http {
> map $http_host $tp_x_forwarded_for {
> default "a";
> }
> log_format main '$ht $tp_x_forwarded_for';
> access_log logs/access.log main;
> server {
> }
> }
> <>
>
> That is because parameter might be allocated one after another and then
> ngx_strncmp would compare whole set and get it wrong. Generally every line
> where there is ngx_strncmp(<typeof ngx_str_t>.data, <char *>, len) where
> length of ngx_str_t is less than len might trigger this issue.
>
> Only small configuration change can change allocation patterns and this
> might not work for you.
>
> I don't know if there is any other usage for this than to learn how nginx
> memory allocators and ngx_str_t works.
>
> If you use no-pool-nginx patch and ASAN, you'll trigger "ERROR:
> AddressSanitizer: heap-buffer-overflow".
>
> Right way to handle this would always check that ngx_str_t len is >=
> ngx_strncmp second arg before every ngx_strncmp()==0 call.
The ngx_strncmp() call without any additional checks is fine as
long as the string tested is expected to be null-terminated (or
followed by some other character known not to match). And this is
the case for most uses of ngx_strncmp(), except may be some bugs.
> Example:
>
> Breakpoint 1, ngx_http_variables_init_vars (cf=cf at entry=0x7fffffffe2b0) at
> src/http/ngx_http_variables.c:2538
> 2538 if (ngx_strncmp(v[i].name.data, "http_", 5) == 0) {
> (gdb) list
> 2533
> 2534 goto next;
> 2535 }
> 2536 }
> 2537
> 2538 if (ngx_strncmp(v[i].name.data, "http_", 5) == 0) {
> 2539 v[i].get_handler = ngx_http_variable_unknown_header_in;
> 2540 v[i].data = (uintptr_t) &v[i].name;
> 2541
> 2542 continue;
> (gdb) print ((ngx_http_variable_t*)cmcf->variables.elts)[i]
> $10 = {name = {len = 2, data = 0x5555558b96a8
> "http_x_forwarded_foraccess_log"}, set_handler = 0x0,
> get_handler = 0x0, data = 0, flags = 0, index = 2}
> gdb) cont
> Continuing.
> nginx: the configuration file /data/nginx/test.conf syntax is ok
> nginx: configuration file /data/nginx/test.conf test is successful
> [Inferior 1 (process 29905) exited normally]
This looks like a bug.
--
Maxim Dounin
http://nginx.org/