Prevent buffer overrun on NGX_HTTP_REQUEST_HEADER_TOO_LARGE

Bondarev, Daniil bondarev at amazon.com
Tue Oct 7 17:23:57 UTC 2014


Hi,

There is an issue in nginx, when it's returning NGX_HTTP_REQUEST_HEADER_TOO_LARGE
in ngx_http_process_request_headers:
When large header buffer is full and the last header size is 1 or 2 bytes more
than NGX_MAX_ERROR_STR - 300, nginx will write 1 or 2 '.' symbols out of large
header buffer, causing unpredictable behavior.

To reproduce it you can send request with total headers size of 'large client buffers size' and
the last header size of 1749. Valgrind will catch this issue:

==10776== Invalid write of size 1                                                                                                                                                                                                                                              
==10776==    at 0x426E84: ngx_http_process_request_headers (ngx_http_request.c:1230)                                                                                                                                                                                           
...

The following patch fixes this issue:

# HG changeset patch
# User Daniil Bondarev <bondarev at amazon.com>
# Date 1412401143 25200
# Node ID 2bbb5284ca7ff658ad50254fe0c5bec14247ba75
# Parent  6bbad2e732458bf53771e80c63a654b3d7f61963
Prevent buffer overrun on NGX_HTTP_REQUEST_HEADER_TOO_LARGE

When large header buffer is full and the last header size is 1 or 2 bytes more
than NGX_MAX_ERROR_STR - 300, nginx will write 1 or 2 '.' symbols out of large
header buffer, causing unpredictable behavior.

The fix is, instead of modifying a buffer, just cut the header and print '...'
in log line if header is too large.

diff -r 6bbad2e73245 -r 2bbb5284ca7f src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c       Wed Aug 27 20:51:01 2014 +0400
+++ b/src/http/ngx_http_request.c       Fri Oct 03 22:39:03 2014 -0700
@@ -1171,6 +1171,7 @@
 {
     u_char                     *p;
     size_t                      len;
+    size_t                      print_len;
     ssize_t                     n;
     ngx_int_t                   rc, rv;
     ngx_table_elt_t            *h;
@@ -1225,14 +1226,13 @@
 
                     len = r->header_in->end - p;
 
-                    if (len > NGX_MAX_ERROR_STR - 300) {
-                        len = NGX_MAX_ERROR_STR - 300;
-                        p[len++] = '.'; p[len++] = '.'; p[len++] = '.';
-                    }
+                    /* Since log line size is limited to NGX_MAX_ERROR_STR,
+                     * nginx has to limit header size it will print into log. */
+                    print_len = ngx_min(len, NGX_MAX_ERROR_STR - 300);
 
                     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%s\"",
+                                  print_len, p, len != print_len ? "..." : "");



More information about the nginx-devel mailing list