Prevent buffer overrun on NGX_HTTP_REQUEST_HEADER_TOO_LARGE

Maxim Dounin mdounin at mdounin.ru
Tue Oct 7 18:57:44 UTC 2014


Hello!

On Tue, Oct 07, 2014 at 06:10:04PM +0000, Bondarev, Daniil wrote:

> Yep, I've thought about this, but prefer not to modify buffer at all, since it feels more error-prone.
> F.E: SB might decide to change number of dots, or reuse last header from this buffer, etc.

Changing the number of dots is highly unlikely and it will be hard 
to do it incorrectly, as the "3" in the patch directly corresponds 
to the number.  Reuse of the header is highly unlikely too, as it 
is the fatal error and the header is known to be incomplete.

> Do you feel strongly against printing "..."  just at log line?

Resulting code is way longer than it should be, so I would rather 
prefer simplier variant.

On the other hand, looking into this more closely, I tend to think 
that ellipsis should be always added to make it clear that the 
header logged is incomplete.

Here is a patch:

--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -1227,12 +1227,11 @@ ngx_http_process_request_headers(ngx_eve
 
                     if (len > NGX_MAX_ERROR_STR - 300) {
                         len = NGX_MAX_ERROR_STR - 300;
-                        p[len++] = '.'; p[len++] = '.'; p[len++] = '.';
                     }
 
                     ngx_log_error(NGX_LOG_INFO, c->log, 0,
-                                  "client sent too long header line: \"%*s\"",
-                                  len, r->header_name_start);
+                                "client sent too long header line: \"%*s...\"",
+                                len, r->header_name_start);
 
                     ngx_http_finalize_request(r,
                                             NGX_HTTP_REQUEST_HEADER_TOO_LARGE);

-- 
Maxim Dounin
http://nginx.org/



More information about the nginx-devel mailing list